| Message ID | 20211226143439.3985960-11-ruansy.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fsdax: introduce fs query to support reflink | expand |
On Sun, Dec 26, 2021 at 10:34:39PM +0800, Shiyang Ruan wrote: > +#define FS_DAX_MAPPING_COW 1UL > + > +#define MAPPING_SET_COW(m) (m = (struct address_space *)FS_DAX_MAPPING_COW) > +#define MAPPING_TEST_COW(m) (((unsigned long)m & FS_DAX_MAPPING_COW) == \ > + FS_DAX_MAPPING_COW) These really should be inline functions and probably use lower case names. But different question, how does this not conflict with: #define PAGE_MAPPING_ANON 0x1 in page-flags.h? Either way I think this flag should move to page-flags.h and be integrated with the PAGE_MAPPING_FLAGS infrastucture.
在 2022/1/20 16:59, Christoph Hellwig 写道: > On Sun, Dec 26, 2021 at 10:34:39PM +0800, Shiyang Ruan wrote: >> +#define FS_DAX_MAPPING_COW 1UL >> + >> +#define MAPPING_SET_COW(m) (m = (struct address_space *)FS_DAX_MAPPING_COW) >> +#define MAPPING_TEST_COW(m) (((unsigned long)m & FS_DAX_MAPPING_COW) == \ >> + FS_DAX_MAPPING_COW) > > These really should be inline functions and probably use lower case > names. OK. > > But different question, how does this not conflict with: > > #define PAGE_MAPPING_ANON 0x1 > > in page-flags.h? Now we are treating dax pages, so I think its flags should be different from normal page. In another word, PAGE_MAPPING_ANON is a flag of rmap mechanism for normal page, it doesn't work for dax page. And now, we have dax rmap for dax page. So, I think this two kinds of flags are supposed to be used in different mechanisms and won't conflect. > > Either way I think this flag should move to page-flags.h and be > integrated with the PAGE_MAPPING_FLAGS infrastucture. And that's why I keep them in this dax.c file. -- Thanks, Ruan.
On Fri, Jan 21, 2022 at 10:33:58AM +0800, Shiyang Ruan wrote: > > > > But different question, how does this not conflict with: > > > > #define PAGE_MAPPING_ANON 0x1 > > > > in page-flags.h? > > Now we are treating dax pages, so I think its flags should be different from > normal page. In another word, PAGE_MAPPING_ANON is a flag of rmap mechanism > for normal page, it doesn't work for dax page. And now, we have dax rmap > for dax page. So, I think this two kinds of flags are supposed to be used > in different mechanisms and won't conflect. It just needs someone to use folio_test_anon in a place where a DAX folio can be passed. This probably should not happen, but we need to clearly document that. > > Either way I think this flag should move to page-flags.h and be > > integrated with the PAGE_MAPPING_FLAGS infrastucture. > > And that's why I keep them in this dax.c file. But that does not integrate it with the infrastructure. For people to debug things it needs to be next to PAGE_MAPPING_ANON and have documentation explaining why they are exclusive.
在 2022/1/21 15:16, Christoph Hellwig 写道: > On Fri, Jan 21, 2022 at 10:33:58AM +0800, Shiyang Ruan wrote: >>> >>> But different question, how does this not conflict with: >>> >>> #define PAGE_MAPPING_ANON 0x1 >>> >>> in page-flags.h? >> >> Now we are treating dax pages, so I think its flags should be different from >> normal page. In another word, PAGE_MAPPING_ANON is a flag of rmap mechanism >> for normal page, it doesn't work for dax page. And now, we have dax rmap >> for dax page. So, I think this two kinds of flags are supposed to be used >> in different mechanisms and won't conflect. > > It just needs someone to use folio_test_anon in a place where a DAX > folio can be passed. This probably should not happen, but we need to > clearly document that. > >>> Either way I think this flag should move to page-flags.h and be >>> integrated with the PAGE_MAPPING_FLAGS infrastucture. >> >> And that's why I keep them in this dax.c file. > > But that does not integrate it with the infrastructure. For people > to debug things it needs to be next to PAGE_MAPPING_ANON and have > documentation explaining why they are exclusive. Ok, understood. -- Thanks, Ruan.
diff --git a/fs/dax.c b/fs/dax.c index ad8ceea1f54c..e72ec712c002 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -335,12 +335,46 @@ static unsigned long dax_end_pfn(void *entry) pfn < dax_end_pfn(entry); pfn++) /* - * TODO: for reflink+dax we need a way to associate a single page with - * multiple address_space instances at different linear_page_index() - * offsets. + * Set FS_DAX_MAPPING_COW flag on the last bit of page->mapping to indicate that + * this is a reflink case. In this case, we associate this page->mapping with + * file mapping at the first time and only once. + */ +#define FS_DAX_MAPPING_COW 1UL + +#define MAPPING_SET_COW(m) (m = (struct address_space *)FS_DAX_MAPPING_COW) +#define MAPPING_TEST_COW(m) (((unsigned long)m & FS_DAX_MAPPING_COW) == \ + FS_DAX_MAPPING_COW) + +/* + * Set or Update the page->mapping with FS_DAX_MAPPING_COW flag. + * Return true if it is an Update. + */ +static inline bool dax_mapping_set_cow(struct page *page) +{ + if (page->mapping) { + /* flag already set */ + if (MAPPING_TEST_COW(page->mapping)) + return false; + + /* + * This page has been mapped even before it is shared, just + * need to set this FS_DAX_MAPPING_COW flag. + */ + MAPPING_SET_COW(page->mapping); + return true; + } + /* Newly associate CoW mapping */ + MAPPING_SET_COW(page->mapping); + return false; +} + +/* + * When it is called in dax_insert_entry(), the cow flag will indicate that + * whether this entry is shared by multiple files. If so, set the page->mapping + * to be FS_DAX_MAPPING_COW, and use page->index as refcount. */ static void dax_associate_entry(void *entry, struct address_space *mapping, - struct vm_area_struct *vma, unsigned long address) + struct vm_area_struct *vma, unsigned long address, bool cow) { unsigned long size = dax_entry_size(entry), pfn, index; int i = 0; @@ -352,9 +386,17 @@ static void dax_associate_entry(void *entry, struct address_space *mapping, for_each_mapped_pfn(entry, pfn) { struct page *page = pfn_to_page(pfn); - WARN_ON_ONCE(page->mapping); - page->mapping = mapping; - page->index = index + i++; + if (cow) { + if (dax_mapping_set_cow(page)) { + /* Was normal, now updated to CoW */ + page->index = 2; + } else + page->index++; + } else { + WARN_ON_ONCE(page->mapping); + page->mapping = mapping; + page->index = index + i++; + } } } @@ -370,7 +412,12 @@ static void dax_disassociate_entry(void *entry, struct address_space *mapping, struct page *page = pfn_to_page(pfn); WARN_ON_ONCE(trunc && page_ref_count(page) > 1); - WARN_ON_ONCE(page->mapping && page->mapping != mapping); + if (!MAPPING_TEST_COW(page->mapping)) { + /* keep the CoW flag if this page is still shared */ + if (page->index-- > 0) + continue; + } else + WARN_ON_ONCE(page->mapping && page->mapping != mapping); page->mapping = NULL; page->index = 0; } @@ -829,7 +876,8 @@ static void *dax_insert_entry(struct xa_state *xas, void *old; dax_disassociate_entry(entry, mapping, false); - dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address); + dax_associate_entry(new_entry, mapping, vmf->vma, vmf->address, + false); /* * Only swap our new entry into the page cache if the current * entry is a zero page or an empty entry. If a normal PTE or
Introduce a FS_DAX_MAPPING_COW flag to support association with CoW file mappings. In this case, the dax-RMAP already takes the responsibility to look up for shared files by given dax page. The page->mapping is no longer to used for rmap but for marking that this dax page is shared. And to make sure disassociation works fine, we use page->index as refcount, and clear page->mapping to the initial state when page->index is decreased to 0. With the help of this new flag, it is able to distinguish normal case and CoW case, and keep the warning in normal case. Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> --- fs/dax.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 57 insertions(+), 9 deletions(-)