Thread (107 messages) 107 messages, 7 authors, 2025-09-09

Re: [PATCH v2 17/54] fs: remove the inode from the LRU list on unlink/rmdir

From: Christian Brauner <brauner@kernel.org>
Date: 2025-08-28 09:00:35
Also in: linux-btrfs, linux-fsdevel, linux-xfs

On Tue, Aug 26, 2025 at 11:39:17AM -0400, Josef Bacik wrote:
quoted hunk ↗ jump to hunk
We can end up with an inode on the LRU list or the cached list, then at
some point in the future go to unlink that inode and then still have an
elevated i_count reference for that inode because it is on one of these
lists.

The more common case is the cached list. We open a file, write to it,
truncate some of it which triggers the inode_add_lru code in the
pagecache, adding it to the cached LRU.  Then we unlink this inode, and
it exists until writeback or reclaim kicks in and removes the inode.

To handle this case, delete the inode from the LRU list when it is
unlinked, so we have the best case scenario for immediately freeing the
inode.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/namei.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)
diff --git a/fs/namei.c b/fs/namei.c
index 138a693c2346..e56dcb5747e4 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -4438,6 +4438,7 @@ SYSCALL_DEFINE2(mkdir, const char __user *, pathname, umode_t, mode)
 int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
 		     struct dentry *dentry)
 {
+	struct inode *inode = dentry->d_inode;
 	int error = may_delete(idmap, dir, dentry, 1);
 
 	if (error)
@@ -4447,11 +4448,11 @@ int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
 		return -EPERM;
 
 	dget(dentry);
-	inode_lock(dentry->d_inode);
+	inode_lock(inode);
 
 	error = -EBUSY;
 	if (is_local_mountpoint(dentry) ||
-	    (dentry->d_inode->i_flags & S_KERNEL_FILE))
+	    (inode->i_flags & S_KERNEL_FILE))
 		goto out;
 
 	error = security_inode_rmdir(dir, dentry);
@@ -4463,12 +4464,21 @@ int vfs_rmdir(struct mnt_idmap *idmap, struct inode *dir,
 		goto out;
 
 	shrink_dcache_parent(dentry);
-	dentry->d_inode->i_flags |= S_DEAD;
+	inode->i_flags |= S_DEAD;
 	dont_mount(dentry);
 	detach_mounts(dentry);
 
 out:
-	inode_unlock(dentry->d_inode);
+	/*
+	 * The inode may be on the LRU list, so delete it from the LRU at this
+	 * point in order to make sure that the inode is freed as soon as
+	 * possible.
+	 */
+	spin_lock(&inode->i_lock);
+	inode_lru_list_del(inode);
+	spin_unlock(&inode->i_lock);
I think it should be possible to optimize this with an appropriate
helper doing:

static inline bool inode_on_lru(const struct inode *inode)
{
	return !!(READ_ONCE(inode->i_state) & (I_LRU | I_CACHED_LRU));
}

then

if (inode_on_lru(inode)) {
	spin_lock(&inode->i_lock);
	inode_lru_list_del(inode);
	spin_unlock(&inode->i_lock);
}

so you don't needlessly acquire i_lock.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help