[PATCH] ext2: take invalidate_lock in ext2_write_failed()

[Pedro Falcato]

Currently, ext2_write_failed() takes care of removing extra data
(folios & blocks) from the page cache by first truncating the page cache,
then truncating the actual inode's blocks.

Understanding that this is correct - namely, that a concurrent page cache
lookup can't create stale pages and stale block mappings between
truncate_pagecache() and ext2_truncate_blocks() - is non trivial. This is
correct for two non-obvious reasons:

1) every caller of ext2_write_failed() holds some sort of exclusion
against truncation. In most cases, callers hold (in some way) the inode
lock. ext2_write_failed() is only reachable from buffered/direct IO write
paths, which hold it. page_symlink() can also call write_begin _without
the inode lock_, but at that point the inode is I_NEW, so there are no other
users.

2) the page cache is generally extremely careful at never instantiating
pages beyond EOF.

This is all quite confusing. Holding the invalidate lock across both
operations is largely simpler, and mirrors other bits of other filesystems
(like ext4_truncate_failed_write). Do that.

This has the side effect of making it so ext2_evict_inode() doesn't take
the invalidate_lock when truncating blocks on inode deletion. This is safe
as the inode should not have any users at that point.

Signed-off-by: Pedro Falcato <pfalcato@xxxxxxx>
---

This is a patch that fell out of trying to understand whether
20261021162105.62525-1-matheus.aalmeida@xxxxxxxxxxxx was correct. I think
it is, for the same reason ext2 is correct. I find this to be easier to reason
about if we just take the invalidate lock across both operations.

 fs/ext2/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/ext2/inode.c b/fs/ext2/inode.c
index 12ac4cfe1500..723b31dc7dac 100644
--- a/fs/ext2/inode.c
+++ b/fs/ext2/inode.c
@@ -58,8 +58,10 @@ void ext2_write_failed(struct address_space *mapping, loff_t to)
 	struct inode *inode = mapping->host;
 
 	if (to > inode->i_size) {
+		filemap_invalidate_lock(mapping);
 		truncate_pagecache(inode, inode->i_size);
 		ext2_truncate_blocks(inode, inode->i_size);
+		filemap_invalidate_unlock(mapping);
 	}
 }
 
@@ -1236,9 +1238,7 @@ static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
 	if (ext2_inode_is_fast_symlink(inode))
 		return;
 
-	filemap_invalidate_lock(inode->i_mapping);
 	__ext2_truncate_blocks(inode, offset);
-	filemap_invalidate_unlock(inode->i_mapping);
 }
 
 static int ext2_setsize(struct inode *inode, loff_t newsize)
-- 
2.56.0