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

["Zi Yan" <ziy@nvidia.com>]

On Sat Nov 7, 2026 at 4:43 PM EST,  wrote:
> Thank you for your contribution! Sashiko AI review found 4 potential
> issues to consider:
> - [High] mm/truncate: unmap full DAX_PMD range in
>   invalidate_inode_pages2_range()
> - [High] fs/dax: premature disassociation of shared folios in
>   dax_can_disassociate()
> - [High] fs/dax: stale i_pages entry when __dax_invalidate_entry() skips
>   busy folio
> - [Medium] dax: cross-file TOCTOU and data race on folio->share in
>   dax_can_disassociate()
> --
>
> commit cd725b6545b68d39f6ee84c70fd91581edad91be
> Author: Zi Yan <ziy@nvidia.com>
>
> mm/truncate: unmap DAX entries before invalidating them
>
> This commit unmaps DAX ranges before invalidating entries in
> invalidate_inode_pages2_range() and skips disassociating clean DAX entries
> whose last association still has elevated folio references.
>
>> 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);
>> +}
>
> [Severity: High]
> Can returning true whenever folio->share > 1 allow a shared PMD folio to be
> dissolved while it is still PMD-mapped?
>

This is related to the issue 1 I mentioned in the patch coment area.

>
> [Severity: Medium]
> Can concurrent invalidations on two files sharing a folio race on
> folio->share in dax_can_disassociate()?

This is the issue 3 I mentioned in the patch comment area.

>
>> @@ -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);
>
> [Severity: High]
> What happens when dax_can_disassociate() returns false during a CoW write and
> leaves the old DAX entry in mapping->i_pages?
>

This is related to the issue 1 I mentioned in the patch comment area.

>
>> 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;
>
> [Severity: High]
> Does unmapping only [start, end] leave PTE mappings in place when [start, end]
> partially overlaps an order-9 DAX_PMD entry in mapping->i_pages?
>

It sounds like it is similar to the issue 4, where PTE mapping and PMD
mapping are not properly handled. I am not sure how PTE mappings could
overlap with an DAX_PMD entry.


It will take me some time to figure out proper fixes for all these
issues. Feel free to take them over if you have more confidence on the
code base.

-- 
Best Regards,
Yan, Zi