| Message ID | 20210924130959.2695749-9-ruansy.fnst@fujitsu.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fsdax: introduce fs query to support reflink | expand |
On Fri, Sep 24, 2021 at 09:09:59PM +0800, Shiyang Ruan wrote: > For reflinked files, one dax page may be associated more than once with > different fime mapping and index. It will report warning. Now, since > we have introduced dax-RMAP for this case and also have to keep its > functionality for other filesystems who are not support rmap, I add this > exception here. > > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> > --- > fs/dax.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/fs/dax.c b/fs/dax.c > index 2536c105ec7f..1a57211b1bc9 100644 > --- a/fs/dax.c > +++ b/fs/dax.c > @@ -352,9 +352,10 @@ 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 (!page->mapping) { > + page->mapping = mapping; > + page->index = index + i++; It feels a little dangerous to have page->mapping for shared storage point to an actual address_space when there are really multiple potential address_spaces out there. If the mm or dax folks are ok with doing this this way then I'll live with it, but it seems like you'd want to leave /some/ kind of marker once you know that the page has multiple owners and therefore regular mm rmap via page->mapping won't work. --D > + } > } > } > > @@ -370,9 +371,10 @@ 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); > - page->mapping = NULL; > - page->index = 0; > + if (page->mapping == mapping) { > + page->mapping = NULL; > + page->index = 0; > + } > } > } > > -- > 2.33.0 > > >
On Thu, Oct 14, 2021 at 12:24:50PM -0700, Darrick J. Wong wrote: > It feels a little dangerous to have page->mapping for shared storage > point to an actual address_space when there are really multiple > potential address_spaces out there. If the mm or dax folks are ok with > doing this this way then I'll live with it, but it seems like you'd want > to leave /some/ kind of marker once you know that the page has multiple > owners and therefore regular mm rmap via page->mapping won't work. Yes, I thing poisoning page->mapping for the rmap enabled case seems like a better idea.
diff --git a/fs/dax.c b/fs/dax.c index 2536c105ec7f..1a57211b1bc9 100644 --- a/fs/dax.c +++ b/fs/dax.c @@ -352,9 +352,10 @@ 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 (!page->mapping) { + page->mapping = mapping; + page->index = index + i++; + } } } @@ -370,9 +371,10 @@ 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); - page->mapping = NULL; - page->index = 0; + if (page->mapping == mapping) { + page->mapping = NULL; + page->index = 0; + } } }
For reflinked files, one dax page may be associated more than once with different fime mapping and index. It will report warning. Now, since we have introduced dax-RMAP for this case and also have to keep its functionality for other filesystems who are not support rmap, I add this exception here. Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com> --- fs/dax.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)