[PATCH] mm/truncate: unmap DAX entries before invalidating them

[PATCH] mm/truncate: unmap DAX entries before invalidating them

[Zi Yan]

invalidate_inode_pages2_range() invalidates the clean DAX entries in the
range through dax_invalidate_mapping_entry_sync() and only unmaps the range
afterwards. Dropping the last association of an entry releases its folio
by dissolving the folio into order-0 ones. If the entry is still mapped,
1. dax_folio_put() warns;
2. if the folio order is PMD, folio_remove_rmap_pmd() during the unmap
   afterwards operates on an order-0 folio (the original head page) and
   corrupts the original first tail page metadata.

This is reachable from userspace with a copy-on-write write to a mapped
reflinked file on XFS with DAX:

  WARNING: fs/dax.c:452 at dax_disassociate_entry.isra.0+0x147/0x1c0
  Call Trace:
   __dax_invalidate_entry+0x106/0x320
   invalidate_inode_pages2_range+0x3ce/0x830
   dax_iomap_rw+0x421/0xcc0
   xfs_file_dax_write+0x252/0x9e0
   vfs_write+0x53a/0xe70
   __x64_sys_pwrite64+0x195/0x250
   ...
  WARNING: ./include/linux/rmap.h:351 at folio_remove_rmap_pmd+0x8b2/0xaf0
  Call Trace:
   split_huge_pmd_locked+0x751/0x2350
   __split_huge_pmd+0x1bb/0x310
   ...
   unmap_mapping_pages+0x133/0x150
   invalidate_inode_pages2_range+0x700/0x830
   dax_iomap_rw+0x421/0xcc0
   ...

To fix it, unmap DAX ranges before invalidating them, as the page cache
path does for mapped folios. In __dax_invalidate_entry(), when
invalidating, skip an entry if the code is dropping its last association
but its folio still has outstanding references. -EBUSY is returned for the
skipped entries. The unmap before invalidation works because any remap of
the folio between the unmap and invalidation will bump the folio refcount.

Fixes: 38607c62b34b ("fs/dax: properly refcount fs dax pages")
Cc: stable@vger.kernel.org
Signed-off-by: Zi Yan <ziy@nvidia.com>
---
I found this while auditing the paths that map pages into user page
tables. invalidate_inode_pages2_range() releases the folios of DAX
entries that are still mapped. For a PMD-mapped folio this corrupts what
used to be its first tail page, and a copy-on-write write to a mapped
reflinked file on XFS with DAX is enough to hit it.

In QEMU with emulated NVDIMMs and XFS on an fsdax namespace:

- Copy-on-write with the script below, PTE- and PMD-mapped, with only a
  mapped and with a and b mapped. Before this patch, the WARNs in the
  commit message and a corrupted first tail page in the PMD case. With this
  patch: the mapping of a sees the new data, b keeps the old data, and
  nothing warns, except for item 4 below.

  mkfs.xfs -f -m reflink=1 -d su=2m,sw=1 /dev/pmem1
  mount -o dax=always /dev/pmem1 /mnt && cd /mnt
  xfs_io -f -c "extsize 2m" -c "pwrite -q -S 0x5a 0 4m" -c fsync a
  xfs_io -f -c "reflink a 0 0 4m" b
  (echo "mmap -r 0 4m"; echo "madvise -h 0 4m"; echo "mread 0 4m";
   sleep 4; echo "mread -v 0 16") | xfs_io a &
  sleep 2; xfs_io -c "pwrite -q -S 0xa5 0 8k" -c fsync a; wait

Open issues that this patch does not address. They are races or
already exist without it.

1. A refault, or a short-term or speculative reference, between the
   early unmap and the invalidation keeps a last association in place,
   and invalidate_inode_pages2_range() returns -EBUSY.
   dax_iomap_iter(), dax_unshare_iter() and dax_zero_iter() ignore that
   and write the new extent anyway, which leaves a stale entry. Without
   this patch the same race tears down the referenced folio. Should
   these paths retry, or fail?

2. XFS device pre-removal ignores the result as well, so an entry kept
   because of a short-term reference is not revisited before the
   memmap goes away.

3. folio->share is updated under each file's i_pages lock, so the
   shared-count check here, like dax_folio_put() itself, can race with
   another file dropping its share.

4. With reflink, after a copy-on-write write, a PTE fault can associate
   an order-0 entry with a shared folio that another file still maps
   with a PMD. That trips WARN_ON_ONCE(dax_entry_order(entry) !=
   folio_order(folio)) in dax_associate_entry(), with and without this
   patch.

Based on an LLM's analysis, 1 and 2 can be fixed by holding the invalidate
lock and breaking DAX layouts in the callers; 3 needs a per-folio lock
for ->share; 4 needs a way to share a folio at different granularities,
or no PMD mappings of shared extents as a simple fix.

LLMs were used to analyze the issue, write the patch, and review the code.
---
 fs/dax.c      | 28 ++++++++++++++++++++++++++--
 mm/truncate.c | 16 +++++++++++-----
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/fs/dax.c b/fs/dax.c
index 94ba41273347c..e448552d8080d 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -527,6 +527,24 @@ static struct page *dax_busy_page(void *entry)
 		return NULL;
 }
 
+/*
+ * Determine if an entry can be disassociated. It cannot be if it is its folio's
+ * last association and the folio still has a reference.
+ */
+static bool dax_can_disassociate(void *entry)
+{
+	struct folio *folio;
+
+	if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry))
+		return true;
+
+	folio = dax_to_folio(entry);
+	if (dax_folio_is_shared(folio) && folio->share > 1)
+		return true;
+
+	return !folio_ref_count(folio);
+}
+
 /**
  * dax_lock_folio - Lock the DAX entry corresponding to a folio
  * @folio: The folio whose entry we want to lock
@@ -853,9 +871,14 @@ static int __dax_invalidate_entry(struct address_space *mapping,
 	entry = get_next_unlocked_entry(&xas, 0);
 	if (!entry || WARN_ON_ONCE(!xa_is_value(entry)))
 		goto out;
+	/*
+	 * Removing the last association of a folio releases the folio, so make
+	 * sure the folio has no reference.
+	 */
 	if (!trunc &&
 	    (xas_get_mark(&xas, PAGECACHE_TAG_DIRTY) ||
-	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE)))
+	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE) ||
+	     !dax_can_disassociate(entry)))
 		goto out;
 	dax_disassociate_entry(entry, mapping, trunc);
 	xas_store(&xas, NULL);
@@ -1017,7 +1040,8 @@ void dax_break_layout_final(struct inode *inode)
 EXPORT_SYMBOL_GPL(dax_break_layout_final);
 
 /*
- * Invalidate DAX entry if it is clean.
+ * Invalidate DAX entry if it is clean and dropping it would not release a folio
+ * that is still in use.
  */
 int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
 				      pgoff_t index)
diff --git a/mm/truncate.c b/mm/truncate.c
index 9e9bf029635cf..b03cd99c4bba8 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -732,6 +732,15 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
 	if (mapping_empty(mapping))
 		return 0;
 
+	/*
+	 * Invalidating a DAX entry can release its folio, so unmap DAX entries
+	 * before invalidating them. dax_invalidate_mapping_entry_sync() skips
+	 * the last association of a folio that still has references, e.g.
+	 * because it was faulted in again in the meantime.
+	 */
+	if (dax_mapping(mapping))
+		unmap_mapping_pages(mapping, start, end - start + 1, false);
+
 	folio_batch_init(&fbatch);
 	index = start;
 	while (find_get_entries(mapping, &index, end, &fbatch, indices)) {
@@ -787,11 +796,8 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
 		cond_resched();
 	}
 	/*
-	 * For DAX we invalidate page tables after invalidating page cache.  We
-	 * could invalidate page tables while invalidating each entry however
-	 * that would be expensive. And doing range unmapping before doesn't
-	 * work as we have no cheap way to find whether page cache entry didn't
-	 * get remapped later.
+	 * For DAX, also zap the mappings that were faulted in while we were
+	 * invalidating.
 	 */
 	if (dax_mapping(mapping)) {
 		unmap_mapping_pages(mapping, start, end - start + 1, false);

---
base-commit: 63919467fb5239ab824de6a7a128b882055b24af
change-id: 20261103-fix-dax-invalidate-mapped-d0c2296f5543

Best regards,
--  
Yan, Zi

Re: [PATCH] mm/truncate: unmap DAX entries before invalidating them

[Alistair Popple]

On 2026-11-07 at 08:43 +1100, Zi Yan <ziy@nvidia.com> wrote...
> invalidate_inode_pages2_range() invalidates the clean DAX entries in the
> range through dax_invalidate_mapping_entry_sync() and only unmaps the range
> afterwards. Dropping the last association of an entry releases its folio
> by dissolving the folio into order-0 ones. If the entry is still mapped,
> 1. dax_folio_put() warns;
> 2. if the folio order is PMD, folio_remove_rmap_pmd() during the unmap
>    afterwards operates on an order-0 folio (the original head page) and
>    corrupts the original first tail page metadata.
> 
> This is reachable from userspace with a copy-on-write write to a mapped
> reflinked file on XFS with DAX:
> 
>   WARNING: fs/dax.c:452 at dax_disassociate_entry.isra.0+0x147/0x1c0
>   Call Trace:
>    __dax_invalidate_entry+0x106/0x320
>    invalidate_inode_pages2_range+0x3ce/0x830
>    dax_iomap_rw+0x421/0xcc0
>    xfs_file_dax_write+0x252/0x9e0
>    vfs_write+0x53a/0xe70
>    __x64_sys_pwrite64+0x195/0x250
>    ...
>   WARNING: ./include/linux/rmap.h:351 at folio_remove_rmap_pmd+0x8b2/0xaf0
>   Call Trace:
>    split_huge_pmd_locked+0x751/0x2350
>    __split_huge_pmd+0x1bb/0x310
>    ...
>    unmap_mapping_pages+0x133/0x150
>    invalidate_inode_pages2_range+0x700/0x830
>    dax_iomap_rw+0x421/0xcc0
>    ...
> 
> To fix it, unmap DAX ranges before invalidating them, as the page cache
> path does for mapped folios. In __dax_invalidate_entry(), when
> invalidating, skip an entry if the code is dropping its last association
> but its folio still has outstanding references. -EBUSY is returned for the
> skipped entries. The unmap before invalidation works because any remap of
> the folio between the unmap and invalidation will bump the folio refcount.

A quick check of callers of invalidate_inode_pages2_range() either ignore
the return code or produce a WARN_ON if the invalidation fails. For example
xfs_fs_commit_blocks(). I don't think anything makes that inherently safe
against COW for example.

That's pre-existing of course - but this patch does increase the likelihood of
error. Overall though I think this patch is still an improvement, and based on
code comments my AI agent concluded that these kind of issues are somewhat known
anyway.

> Fixes: 38607c62b34b ("fs/dax: properly refcount fs dax pages")

I suspect this issue actually preexists my changes here. Prior the page
meta-data was still getting modified on disassociation even if the folio was
referenced. In some ways proper refcounting just made it easier to catch these
kind of problems, which was partly the point of doing it :-)

But I think it makes sense to use this as the "Fixes" tag - backporting this to
anything prior would be difficult and a rewrite of this fix anyway.

> Cc: stable@vger.kernel.org
> Signed-off-by: Zi Yan <ziy@nvidia.com>
> ---
> I found this while auditing the paths that map pages into user page
> tables. invalidate_inode_pages2_range() releases the folios of DAX
> entries that are still mapped. For a PMD-mapped folio this corrupts what
> used to be its first tail page, and a copy-on-write write to a mapped
> reflinked file on XFS with DAX is enough to hit it.
> 
> In QEMU with emulated NVDIMMs and XFS on an fsdax namespace:
> 
> - Copy-on-write with the script below, PTE- and PMD-mapped, with only a
>   mapped and with a and b mapped. Before this patch, the WARNs in the
>   commit message and a corrupted first tail page in the PMD case. With this
>   patch: the mapping of a sees the new data, b keeps the old data, and
>   nothing warns, except for item 4 below.
> 
>   mkfs.xfs -f -m reflink=1 -d su=2m,sw=1 /dev/pmem1
>   mount -o dax=always /dev/pmem1 /mnt && cd /mnt
>   xfs_io -f -c "extsize 2m" -c "pwrite -q -S 0x5a 0 4m" -c fsync a
>   xfs_io -f -c "reflink a 0 0 4m" b
>   (echo "mmap -r 0 4m"; echo "madvise -h 0 4m"; echo "mread 0 4m";
>    sleep 4; echo "mread -v 0 16") | xfs_io a &
>   sleep 2; xfs_io -c "pwrite -q -S 0xa5 0 8k" -c fsync a; wait

Ideally I think this should be added to the xfstest suite - that was extremely
useful during some of the refcounting reworks and found all sorts of interesting
corner cases.

> Open issues that this patch does not address. They are races or
> already exist without it.
> 
> 1. A refault, or a short-term or speculative reference, between the
>    early unmap and the invalidation keeps a last association in place,
>    and invalidate_inode_pages2_range() returns -EBUSY.
>    dax_iomap_iter(), dax_unshare_iter() and dax_zero_iter() ignore that
>    and write the new extent anyway, which leaves a stale entry. Without
>    this patch the same race tears down the referenced folio. Should
>    these paths retry, or fail?
> 
> 2. XFS device pre-removal ignores the result as well, so an entry kept
>    because of a short-term reference is not revisited before the
>    memmap goes away.
> 
> 3. folio->share is updated under each file's i_pages lock, so the
>    shared-count check here, like dax_folio_put() itself, can race with
>    another file dropping its share.
> 
> 4. With reflink, after a copy-on-write write, a PTE fault can associate
>    an order-0 entry with a shared folio that another file still maps
>    with a PMD. That trips WARN_ON_ONCE(dax_entry_order(entry) !=
>    folio_order(folio)) in dax_associate_entry(), with and without this
>    patch.
> 
> Based on an LLM's analysis, 1 and 2 can be fixed by holding the invalidate
> lock and breaking DAX layouts in the callers; 3 needs a per-folio lock
> for ->share; 4 needs a way to share a folio at different granularities,
> or no PMD mappings of shared extents as a simple fix.
> 
> LLMs were used to analyze the issue, write the patch, and review the code.
> ---
>  fs/dax.c      | 28 ++++++++++++++++++++++++++--
>  mm/truncate.c | 16 +++++++++++-----
>  2 files changed, 37 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/dax.c b/fs/dax.c
> index 94ba41273347c..e448552d8080d 100644
> --- a/fs/dax.c
> +++ b/fs/dax.c
> @@ -527,6 +527,24 @@ static struct page *dax_busy_page(void *entry)
>  		return NULL;
>  }
>  
> +/*
> + * Determine if an entry can be disassociated. It cannot be if it is its folio's
> + * last association and the folio still has a reference.
> + */
> +static bool dax_can_disassociate(void *entry)
> +{
> +	struct folio *folio;
> +
> +	if (dax_is_zero_entry(entry) || dax_is_empty_entry(entry))
> +		return true;
> +
> +	folio = dax_to_folio(entry);
> +	if (dax_folio_is_shared(folio) && folio->share > 1)
> +		return true;
> +
> +	return !folio_ref_count(folio);
> +}
> +
>  /**
>   * dax_lock_folio - Lock the DAX entry corresponding to a folio
>   * @folio: The folio whose entry we want to lock
> @@ -853,9 +871,14 @@ static int __dax_invalidate_entry(struct address_space *mapping,
>  	entry = get_next_unlocked_entry(&xas, 0);
>  	if (!entry || WARN_ON_ONCE(!xa_is_value(entry)))
>  		goto out;
> +	/*
> +	 * Removing the last association of a folio releases the folio, so make
> +	 * sure the folio has no reference.
> +	 */
>  	if (!trunc &&
>  	    (xas_get_mark(&xas, PAGECACHE_TAG_DIRTY) ||
> -	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE)))
> +	     xas_get_mark(&xas, PAGECACHE_TAG_TOWRITE) ||
> +	     !dax_can_disassociate(entry)))
>  		goto out;
>  	dax_disassociate_entry(entry, mapping, trunc);
>  	xas_store(&xas, NULL);
> @@ -1017,7 +1040,8 @@ void dax_break_layout_final(struct inode *inode)
>  EXPORT_SYMBOL_GPL(dax_break_layout_final);
>  
>  /*
> - * Invalidate DAX entry if it is clean.
> + * Invalidate DAX entry if it is clean and dropping it would not release a folio
> + * that is still in use.
>   */
>  int dax_invalidate_mapping_entry_sync(struct address_space *mapping,
>  				      pgoff_t index)
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 9e9bf029635cf..b03cd99c4bba8 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -732,6 +732,15 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
>  	if (mapping_empty(mapping))
>  		return 0;
>  
> +	/*
> +	 * Invalidating a DAX entry can release its folio, so unmap DAX entries
> +	 * before invalidating them. dax_invalidate_mapping_entry_sync() skips
> +	 * the last association of a folio that still has references, e.g.
> +	 * because it was faulted in again in the meantime.
> +	 */
> +	if (dax_mapping(mapping))
> +		unmap_mapping_pages(mapping, start, end - start + 1, false);
> +
>  	folio_batch_init(&fbatch);
>  	index = start;
>  	while (find_get_entries(mapping, &index, end, &fbatch, indices)) {
> @@ -787,11 +796,8 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
>  		cond_resched();
>  	}
>  	/*
> -	 * For DAX we invalidate page tables after invalidating page cache.  We
> -	 * could invalidate page tables while invalidating each entry however
> -	 * that would be expensive. And doing range unmapping before doesn't
> -	 * work as we have no cheap way to find whether page cache entry didn't
> -	 * get remapped later.
> +	 * For DAX, also zap the mappings that were faulted in while we were
> +	 * invalidating.
>  	 */
>  	if (dax_mapping(mapping)) {
>  		unmap_mapping_pages(mapping, start, end - start + 1, false);
> 
> ---
> base-commit: 63919467fb5239ab824de6a7a128b882055b24af
> change-id: 20261103-fix-dax-invalidate-mapped-d0c2296f5543
> 
> Best regards,
> --  
> Yan, Zi
>