lkml.org 
[lkml]   [2022]   [Dec]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRE: [PATCH v2.1 1/8] fsdax: introduce page->share for fsdax in reflink mode
    Shiyang Ruan wrote:
    > fsdax page is used not only when CoW, but also mapread. To make the it
    > easily understood, use 'share' to indicate that the dax page is shared
    > by more than one extent. And add helper functions to use it.
    >
    > Also, the flag needs to be renamed to PAGE_MAPPING_DAX_SHARED.
    >
    > Signed-off-by: Shiyang Ruan <ruansy.fnst@fujitsu.com>
    > ---
    > fs/dax.c | 38 ++++++++++++++++++++++----------------
    > include/linux/mm_types.h | 5 ++++-
    > include/linux/page-flags.h | 2 +-
    > 3 files changed, 27 insertions(+), 18 deletions(-)
    >
    > diff --git a/fs/dax.c b/fs/dax.c
    > index 1c6867810cbd..edbacb273ab5 100644
    > --- a/fs/dax.c
    > +++ b/fs/dax.c
    > @@ -334,35 +334,41 @@ static unsigned long dax_end_pfn(void *entry)
    > for (pfn = dax_to_pfn(entry); \
    > pfn < dax_end_pfn(entry); pfn++)
    >
    > -static inline bool dax_mapping_is_cow(struct address_space *mapping)
    > +static inline bool dax_page_is_shared(struct page *page)
    > {
    > - return (unsigned long)mapping == PAGE_MAPPING_DAX_COW;
    > + return (unsigned long)page->mapping == PAGE_MAPPING_DAX_SHARED;
    > }
    >
    > /*
    > - * Set the page->mapping with FS_DAX_MAPPING_COW flag, increase the refcount.
    > + * Set the page->mapping with PAGE_MAPPING_DAX_SHARED flag, increase the
    > + * refcount.
    > */
    > -static inline void dax_mapping_set_cow(struct page *page)
    > +static inline void dax_page_bump_sharing(struct page *page)

    Similar to page_ref naming I would call this page_share_get() and the
    corresponding function page_share_put().

    > {
    > - if ((uintptr_t)page->mapping != PAGE_MAPPING_DAX_COW) {
    > + if ((uintptr_t)page->mapping != PAGE_MAPPING_DAX_SHARED) {
    > /*
    > * Reset the index if the page was already mapped
    > * regularly before.
    > */
    > if (page->mapping)
    > - page->index = 1;
    > - page->mapping = (void *)PAGE_MAPPING_DAX_COW;
    > + page->share = 1;
    > + page->mapping = (void *)PAGE_MAPPING_DAX_SHARED;

    Small nit, You could save a cast here by defining
    PAGE_MAPPING_DAX_SHARED as "((void *) 1)".

    > }
    > - page->index++;
    > + page->share++;
    > +}
    > +
    > +static inline unsigned long dax_page_drop_sharing(struct page *page)
    > +{
    > + return --page->share;
    > }
    >
    > /*
    > - * When it is called in dax_insert_entry(), the cow flag will indicate that
    > + * When it is called in dax_insert_entry(), the shared flag will indicate that
    > * whether this entry is shared by multiple files. If so, set the page->mapping
    > - * FS_DAX_MAPPING_COW, and use page->index as refcount.
    > + * PAGE_MAPPING_DAX_SHARED, and use page->share as refcount.
    > */
    > static void dax_associate_entry(void *entry, struct address_space *mapping,
    > - struct vm_area_struct *vma, unsigned long address, bool cow)
    > + struct vm_area_struct *vma, unsigned long address, bool shared)
    > {
    > unsigned long size = dax_entry_size(entry), pfn, index;
    > int i = 0;
    > @@ -374,8 +380,8 @@ static void dax_associate_entry(void *entry, struct address_space *mapping,
    > for_each_mapped_pfn(entry, pfn) {
    > struct page *page = pfn_to_page(pfn);
    >
    > - if (cow) {
    > - dax_mapping_set_cow(page);
    > + if (shared) {
    > + dax_page_bump_sharing(page);
    > } else {
    > WARN_ON_ONCE(page->mapping);
    > page->mapping = mapping;
    > @@ -396,9 +402,9 @@ 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);
    > - if (dax_mapping_is_cow(page->mapping)) {
    > - /* keep the CoW flag if this page is still shared */
    > - if (page->index-- > 0)
    > + if (dax_page_is_shared(page)) {
    > + /* keep the shared flag if this page is still shared */
    > + if (dax_page_drop_sharing(page) > 0)
    > continue;

    I think part of what makes this hard to read is trying to preserve the
    same code paths for shared pages and typical pages.

    page_share_put() should, in addition to decrementing the share, clear
    out page->mapping value.

    > } else
    > WARN_ON_ONCE(page->mapping && page->mapping != mapping);
    > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
    > index 500e536796ca..f46cac3657ad 100644
    > --- a/include/linux/mm_types.h
    > +++ b/include/linux/mm_types.h
    > @@ -103,7 +103,10 @@ struct page {
    > };
    > /* See page-flags.h for PAGE_MAPPING_FLAGS */
    > struct address_space *mapping;
    > - pgoff_t index; /* Our offset within mapping. */
    > + union {
    > + pgoff_t index; /* Our offset within mapping. */
    > + unsigned long share; /* share count for fsdax */
    > + };
    > /**
    > * @private: Mapping-private opaque data.
    > * Usually used for buffer_heads if PagePrivate.
    > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
    > index 0b0ae5084e60..c8a3aa02278d 100644
    > --- a/include/linux/page-flags.h
    > +++ b/include/linux/page-flags.h
    > @@ -641,7 +641,7 @@ PAGEFLAG_FALSE(VmemmapSelfHosted, vmemmap_self_hosted)
    > * Different with flags above, this flag is used only for fsdax mode. It
    > * indicates that this page->mapping is now under reflink case.
    > */
    > -#define PAGE_MAPPING_DAX_COW 0x1
    > +#define PAGE_MAPPING_DAX_SHARED 0x1
    >
    > static __always_inline bool folio_mapping_flags(struct folio *folio)
    > {
    > --
    > 2.38.1
    >


    \
     
     \ /
      Last update: 2022-12-03 03:09    [W:5.300 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site