[PATCH 05/46] mm/truncate: look again if a folio left the mapping

[David Hildenbrand (Arm)]

invalidate_inode_pages2_range() skips a folio which no longer belongs to
the mapping by the time it has got the folio lock, on the assumption
that somebody else has removed it from the page cache. But the folio may
have been replaced, and then there is a new folio with the same data at
that index.

For most cases, like folio migration, that is impossible. However,
collapse_file() only checks the refcount of the old folios, under the
i_pages lock, and relies on their folio locks from there on. That works for
users which repeat the lookup when they find folio->mapping changed.
The following scenario is, however, possible:

  CPU0 pwrite(O_DIRECT)               CPU1 collapse_file()
  kiocb_invalidate_pages()
                                      read the folio (old data), lock it
                                      refcount check
  the write completes                 copy to new_folio
  kiocb_invalidate_post_direct_write()
    find_get_entries() -> old folio
    folio_lock() ...                  xas_store(new_folio)
                                      folio->mapping = NULL
                                      folio_unlock(folio)
    folio->mapping != mapping
      -> continue

In that scenario, the collapsed folio with the data from before the write
stays in the page cache after the write has returned.

This used to be impossible, as khugepaged only collapsed files that nobody
had open for writing.

Restart the lookup at that index, like shmem_undo_range() does. The same
problem seems to exist in truncate_inode_pages_range() as well when
truncate_inode_folio() fails, so handle it there as well.

Fixes: 1b6444331beb ("mm/khugepaged: enable clean pagecache folio collapse for writable files")
Cc: stable at vger.kernel.org
Reported-by: Josef Bacik <josef at toxicpanda.com>
Signed-off-by: David Hildenbrand (Arm) <david at kernel.org>
---
 mm/truncate.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/mm/truncate.c b/mm/truncate.c
index 733d2713a875a..6d8c0ec4623fe 100644
--- a/mm/truncate.c
+++ b/mm/truncate.c
@@ -499,7 +499,15 @@ void truncate_inode_pages_range(struct address_space *mapping,
 			folio_lock(folio);
 			VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio);
 			folio_wait_writeback(folio);
-			truncate_inode_folio(mapping, folio);
+			if (truncate_inode_folio(mapping, folio)) {
+				folio_unlock(folio);
+				/*
+				 * Folio might have been replaced: look it up
+				 * again.
+				 */
+				index = indices[i];
+				break;
+			}
 			folio_unlock(folio);
 		}
 		truncate_folio_batch_exceptionals(mapping, &fbatch, indices);
@@ -760,7 +768,12 @@ int invalidate_inode_pages2_range(struct address_space *mapping,
 			folio_lock(folio);
 			if (unlikely(folio->mapping != mapping)) {
 				folio_unlock(folio);
-				continue;
+				/*
+				 * Folio might have been replaced: look up
+				 * again.
+				 */
+				index = indices[i];
+				break;
 			}
 			VM_BUG_ON_FOLIO(!folio_contains(folio, indices[i]), folio);
 			folio_wait_writeback(folio);

-- 
2.43.0