Re: [PATCH v10 9/9] fsdax: set a CoW flag when associate reflink mappings
On Tue, Feb 15, 2022 at 6:55 PM Shiyang Ruan wrote: > > > > 在 2022/2/16 10:09, Dan Williams 写道: > > On Thu, Jan 27, 2022 at 4:41 AM Shiyang Ruan > > wrote: > >> > >> Introduce a PAGE_MAPPING_DAX_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 > >> --- > >> fs/dax.c | 65 -- > >> include/linux/page-flags.h | 6 > >> 2 files changed, 62 insertions(+), 9 deletions(-) > >> > >> diff --git a/fs/dax.c b/fs/dax.c > >> index 250794a5b789..88879c579c1f 100644 > >> --- a/fs/dax.c > >> +++ b/fs/dax.c > >> @@ -334,13 +334,46 @@ static unsigned long dax_end_pfn(void *entry) > >> for (pfn = dax_to_pfn(entry); \ > >> pfn < dax_end_pfn(entry); pfn++) > >> > >> +static inline void dax_mapping_set_cow_flag(struct address_space *mapping) > >> +{ > >> + mapping = (struct address_space *)PAGE_MAPPING_DAX_COW; > >> +} > >> + > >> +static inline bool dax_mapping_is_cow(struct address_space *mapping) > >> +{ > >> + return (unsigned long)mapping == PAGE_MAPPING_DAX_COW; > >> +} > >> + > >> /* > >> - * 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 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 (dax_mapping_is_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. > >> +*/ > >> + dax_mapping_set_cow_flag(page->mapping); > >> + return true; > >> + } > >> + /* Newly associate CoW mapping */ > >> + dax_mapping_set_cow_flag(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 +385,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 +411,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 (!dax_mapping_is_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;
Re: [PATCH v10 9/9] fsdax: set a CoW flag when associate reflink mappings
在 2022/2/16 10:09, Dan Williams 写道: On Thu, Jan 27, 2022 at 4:41 AM Shiyang Ruan wrote: Introduce a PAGE_MAPPING_DAX_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 --- fs/dax.c | 65 -- include/linux/page-flags.h | 6 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/fs/dax.c b/fs/dax.c index 250794a5b789..88879c579c1f 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -334,13 +334,46 @@ static unsigned long dax_end_pfn(void *entry) for (pfn = dax_to_pfn(entry); \ pfn < dax_end_pfn(entry); pfn++) +static inline void dax_mapping_set_cow_flag(struct address_space *mapping) +{ + mapping = (struct address_space *)PAGE_MAPPING_DAX_COW; +} + +static inline bool dax_mapping_is_cow(struct address_space *mapping) +{ + return (unsigned long)mapping == PAGE_MAPPING_DAX_COW; +} + /* - * 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 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 (dax_mapping_is_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. +*/ + dax_mapping_set_cow_flag(page->mapping); + return true; + } + /* Newly associate CoW mapping */ + dax_mapping_set_cow_flag(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 +385,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 +411,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 (!dax_mapping_is_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; } @@ -810,7 +856,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); Where is the caller that passes 'true'? Also when that caller arrives introduce a separate dax_associate_cow_entry() as that's easier to read than dax_associate_entry(..., true) in case someone does not remember what that boolean flag means. This flag is
Re: [PATCH v10 9/9] fsdax: set a CoW flag when associate reflink mappings
On Thu, Jan 27, 2022 at 4:41 AM Shiyang Ruan wrote: > > Introduce a PAGE_MAPPING_DAX_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 > --- > fs/dax.c | 65 -- > include/linux/page-flags.h | 6 > 2 files changed, 62 insertions(+), 9 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index 250794a5b789..88879c579c1f 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -334,13 +334,46 @@ static unsigned long dax_end_pfn(void *entry) > for (pfn = dax_to_pfn(entry); \ > pfn < dax_end_pfn(entry); pfn++) > > +static inline void dax_mapping_set_cow_flag(struct address_space *mapping) > +{ > + mapping = (struct address_space *)PAGE_MAPPING_DAX_COW; > +} > + > +static inline bool dax_mapping_is_cow(struct address_space *mapping) > +{ > + return (unsigned long)mapping == PAGE_MAPPING_DAX_COW; > +} > + > /* > - * 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 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 (dax_mapping_is_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. > +*/ > + dax_mapping_set_cow_flag(page->mapping); > + return true; > + } > + /* Newly associate CoW mapping */ > + dax_mapping_set_cow_flag(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 +385,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 +411,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 (!dax_mapping_is_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; > } > @@ -810,7 +856,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); Where is the caller that passes 'true'? Also when that caller arrives introduce a sepa
[PATCH v10 9/9] fsdax: set a CoW flag when associate reflink mappings
Introduce a PAGE_MAPPING_DAX_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 --- fs/dax.c | 65 -- include/linux/page-flags.h | 6 2 files changed, 62 insertions(+), 9 deletions(-) diff --git a/fs/dax.c b/fs/dax.c index 250794a5b789..88879c579c1f 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -334,13 +334,46 @@ static unsigned long dax_end_pfn(void *entry) for (pfn = dax_to_pfn(entry); \ pfn < dax_end_pfn(entry); pfn++) +static inline void dax_mapping_set_cow_flag(struct address_space *mapping) +{ + mapping = (struct address_space *)PAGE_MAPPING_DAX_COW; +} + +static inline bool dax_mapping_is_cow(struct address_space *mapping) +{ + return (unsigned long)mapping == PAGE_MAPPING_DAX_COW; +} + /* - * 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 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 (dax_mapping_is_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. +*/ + dax_mapping_set_cow_flag(page->mapping); + return true; + } + /* Newly associate CoW mapping */ + dax_mapping_set_cow_flag(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 +385,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 +411,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 (!dax_mapping_is_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; } @@ -810,7 +856,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 diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 1c3b6e5c8bfd..6370d279795a 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h @@ -572,6 +572,12 @@