| Message ID | f0b31772-78d7-f198-6482-9f25aab8c13f@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | tmpfs: user xattrs and direct IO | expand |
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 >
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))
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> --- mm/truncate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)