Re: [PATCH v12 7/7] fsdax: set a CoW flag when associate reflink mappings

Christoph Hellwig
2022-04-11T06:55:49Z

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