Re: [PATCH vfs.tmpfs 5/5] mm: invalidation check mapping before folio_contains

[Jan Kara <jack@suse.cz>]

On Tue 08-08-23 21:36:12, Hugh Dickins wrote:
> Enabling tmpfs "direct IO" exposes it to invalidate_inode_pages2_range(),
> which when swapping can hit the VM_BUG_ON_FOLIO(!folio_contains()): the
> folio has been moved from page cache to swap cache (with folio->mapping
> reset to NULL), but the folio_index() embedded in folio_contains() sees
> swapcache, and so returns the swapcache_index() - whereas folio->index
> would be the right one to check against the index from mapping's xarray.
> 
> There are different ways to fix this, but my preference is just to order
> the checks in invalidate_inode_pages2_range() the same way that they are
> in __filemap_get_folio() and find_lock_entries() and filemap_fault():
> check folio->mapping before folio_contains().
> 
> Signed-off-by: Hugh Dickins <hughd@google.com>

Makes sense. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  mm/truncate.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/mm/truncate.c b/mm/truncate.c
> index 95d1291d269b..c3320e66d6ea 100644
> --- a/mm/truncate.c
> +++ b/mm/truncate.c
> @@ -657,11 +657,11 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
>  			}
>  
>  			folio_lock(folio);
> -			VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio);
> -			if (folio->mapping != mapping) {
> +			if (unlikely(folio->mapping != mapping)) {
>  				folio_unlock(folio);
>  				continue;
>  			}
> +			VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio);
>  			folio_wait_writeback(folio);
>  
>  			if (folio_mapped(folio))
> -- 
> 2.35.3
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR