Re: [PATCH] ext2: take invalidate_lock in ext2_write_failed()

[sashiko-bot@kernel.org]

> 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@suse.de>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024222726.889520-1-pfalcato@suse.de?part=1