> + * 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;

Given that this is the only place calling dax_mapping_set_cow I wonder
if we should just open code it here, and also lift the page->index logic
from the caller into this helper.

static inline void dax_mapping_set_cow(struct page *page)
{
        if ((uintptr_t)page->mapping != PAGE_MAPPING_DAX_COW) {
                /*
                 * 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->index++;
}

> +             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);

Isnt the dax_mapping_is_cow check above inverted?

Reply via email to