[PATCH RFC v2 13/13] fs: don't strand I_METADATA_WRITEBACK inodes outside the LRU

[Christian Brauner <brauner@kernel.org>]

Since commit c474bc56b6d1 ("fs: Provide way for filesystem to wait for
metadata writeback") ext2, ext4 without a journal, fat, udf and minix
set I_METADATA_WRITEBACK on an inode whose metadata was copied into a
buffer that might not have reached the disk yet. Only data integrity
writeback, i.e. fsync() and friends, clears it again. The flusher and
sync(2) leave it alone.

inode_lru_isolate() treats every state bit other than I_REFERENCED as a
reason not to reclaim the inode right now. It takes the inode off the
LRU and relies on iput(), writeback completion or the removal of the
last page from the page cache to put it back once the reason is gone.

But I_METADATA_WRITEBACK doesn't go away on its own. iput() puts the
inode back on the LRU with the bit still set and the next scan takes it
off again. So neither memory pressure nor drop_caches can free these
inodes. Only fsync() and friends or unmounting the filesystem get rid
of them.

On ext4 without a journal ext4_mark_iloc_dirty() sets the bit on every
ext4_mark_inode_dirty(), so even an atime update is enough. An
unprivileged user creating or touching lots of files grows the inode
cache without bound until the OOM killer comes along.

The inode can't just be thrown away though. An fsync() after the inode
was read back in wouldn't know that its metadata still has to be
written out. But that's no different from the metadata buffers these
filesystems track in their mapping_metadata_bhs. Their ->evict_inode()
calls mmb_sync() and that is what allowed commit 972b9dd4e418 ("fs:
Ignore inode metadata buffers in inode_lru_isolate()") to take those
buffers out of inode_lru_isolate(). With the previous patches
->evict_inode() writes out what I_METADATA_WRITEBACK tracks the same
way, without reading anything from disk.

So let the shrinker reclaim inodes with I_METADATA_WRITEBACK like any
other clean inode.

Handing these inodes to a work item that syncs them and puts them back
on the LRU would work too. But that's a second inode reclaim offload
mechanism next to the deferred inode reclaim that is being worked on,
and it needs a reference on the inode which leaks into remount, freeze
and umount.

For stable this needs the previous patches as well. Without the
->evict_inode() changes an fsync() after the inode was read back in
misses the inode block. Without the lookup changes ->evict_inode() reads
inode blocks from disk with __GFP_NOFAIL allocations from reclaim. v7.3
also still has bfs which sets I_METADATA_WRITEBACK too, so the backport
needs the same two changes for bfs.

Fixes: c474bc56b6d1 ("fs: Provide way for filesystem to wait for metadata writeback")
Cc: stable@vger.kernel.org # v7.3; see patch description
Reported-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
---
 fs/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/inode.c b/fs/inode.c
index 1cb6293b237a..16ebd686d681 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -947,9 +947,12 @@ static enum lru_status inode_lru_isolate(struct list_head *item,
 	 * they're already on the LRU, and this can make them
 	 * unreclaimable for a while. Remove them lazily here; iput,
 	 * sync, or the last page cache deletion will requeue them.
+	 * Nothing requeues them for I_METADATA_WRITEBACK, ->evict_inode()
+	 * writes that out instead.
 	 */
 	if (icount_read(inode) ||
-	    (inode_state_read(inode) & ~I_REFERENCED) ||
+	    (inode_state_read(inode) &
+	     ~(I_REFERENCED | I_METADATA_WRITEBACK)) ||
 	    !mapping_shrinkable(&inode->i_data)) {
 		list_lru_isolate(lru, &inode->i_lru);
 		spin_unlock(&inode->i_lock);

-- 
2.53.0