[RFC PATCH v2 0/3] fs/proc: split the inode list for procfs

[RFC PATCH v2 0/3] fs/proc: split the inode list for procfs

[Huang Shijie]

The global s_inode_list_lock is heavily contended in procfs
on a 384-CPU, 12-NUMA-node Hygon machine running Hadoop TestDFSIO:
  #hadoop jar xxxx.jar TestDFSIO -read -nrFiles 1000 -size 100MB

The hadoop will create lot of threads during the test.
The perf shows it consuming ~90% of the lock hotspot in procfs.
The lock is hit from both directions:
   -- inode creation (~49%) :
           getdents64 ->
              proc_readfd_common ->
	         new_inode ->
		   inode_sb_list_add()

   -- inode eviction (~41%):
           process exit ->
	      release_task ->
	         proc_invalidate_siblings_dcache ->
		     evict ->
		        inode_sb_list_del()

This patch set tries to resolve this issue by:
    patch 1: "fs/drop_caches: skip filesystems without page cache"
 	add a new flag for filesystems without page cache.
    patch 2: Add a new helper to detect the empty inode list    
    patch 3: split the inode list.


After this patch set, the above TestDFSIO can get over 50% better performance
in exec time. And the procfs lock hotspot becomes nearly 1%.

Huang Shijie (3):
  fs/drop_caches: skip filesystems without page cache
  fs/super: introduce a helper sb_inodes_empty()
  fs/proc: split the inode list

 fs/drop_caches.c               |   3 +
 fs/inode.c                     |  79 ++++++++++------
 fs/proc/inode.c                |  53 +++++++++++
 fs/proc/internal.h             |   2 +
 fs/proc/root.c                 |   8 +-
 fs/super.c                     |  45 +++++++--
 include/linux/fs/super_types.h |  17 ++++
 include/linux/proc_fs.h        |   1 +
 security/landlock/fs.c         | 162 ++++++++++++++++++---------------
 9 files changed, 260 insertions(+), 110 deletions(-)

-- 
2.53.0

[RFC PATCH v2 1/3] fs/drop_caches: skip filesystems without page cache

[Huang Shijie]

Add a new flag SB_I_NO_PAGECACHE for superblock.

Skip scanning the inode lists of filesystems that have no page cache
in drop_pagecache_sb(), as indicated by the SB_I_NO_PAGECACHE flag.

This patch only changes the procfs.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
 fs/drop_caches.c               | 3 +++
 fs/proc/root.c                 | 2 +-
 include/linux/fs/super_types.h | 1 +
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index 49f56a598ecb..27caf748b3c5 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -20,6 +20,9 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused)
 {
 	struct inode *inode, *toput_inode = NULL;
 
+	if (sb->s_iflags & SB_I_NO_PAGECACHE)
+		return;
+
 	spin_lock(&sb->s_inode_list_lock);
 	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
 		spin_lock(&inode->i_lock);
diff --git a/fs/proc/root.c b/fs/proc/root.c
index 99adddfeb4a4..d45f5af5ab53 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -266,7 +266,7 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 		return ret;
 
 	/* User space would break if executables or devices appear on proc */
-	s->s_iflags |= SB_I_NOEXEC | SB_I_NODEV;
+	s->s_iflags |= SB_I_NOEXEC | SB_I_NODEV | SB_I_NO_PAGECACHE;
 	s->s_flags |= SB_NODIRATIME | SB_NOSUID | SB_NOEXEC;
 	s->s_blocksize = 1024;
 	s->s_blocksize_bits = 10;
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index ecd96aeb1cee..1a596caf58a8 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -352,5 +352,6 @@ struct super_block {
 #define SB_I_NOIDMAP	0x00002000	/* No idmapped mounts on this superblock */
 #define SB_I_ALLOW_HSM	0x00004000	/* Allow HSM events on this superblock */
 #define SB_I_NO_DATA_INTEGRITY	0x00008000 /* fs cannot guarantee data persistence on sync */
+#define SB_I_NO_PAGECACHE	0x00010000 /* this file system has no page cache */
 
 #endif /* _LINUX_FS_SUPER_TYPES_H */
-- 
2.53.0

Re: [RFC PATCH v2 1/3] fs/drop_caches: skip filesystems without page cache

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 03:28:09PM +0800, Huang Shijie wrote:
> Add a new flag SB_I_NO_PAGECACHE for superblock.
> 
> Skip scanning the inode lists of filesystems that have no page cache
> in drop_pagecache_sb(), as indicated by the SB_I_NO_PAGECACHE flag.

I don't think we need a new flag.  This can check for

	sb->s_bdi == &noop_backing_dev_info

which is actually the more relevant concept - not if a file system
uses the page cache, but if it supports page cache writeback.
E.g. for ramfs there is page cache, but it is the only data store.
For shmemfs, there is writeback, but it is driven in a different
way (through the anonymous memory code and not fs writeback).

And most importantly it just work, no need to mark file systems,
as you're currently missing a lot of those that can have a fair
number of inodes, e.g., sysfs, kernfs, cgroupfs or configfs.

Re: [RFC PATCH v2 1/3] fs/drop_caches: skip filesystems without page cache

[Huang Shijie]

On Mon, Sep 21, 2026 at 10:41:26PM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 03:28:09PM +0800, Huang Shijie wrote:
> > Add a new flag SB_I_NO_PAGECACHE for superblock.
> > 
> > Skip scanning the inode lists of filesystems that have no page cache
> > in drop_pagecache_sb(), as indicated by the SB_I_NO_PAGECACHE flag.
> 
> I don't think we need a new flag.  This can check for
> 
> 	sb->s_bdi == &noop_backing_dev_info
> 
Okay. I can use this method.

> which is actually the more relevant concept - not if a file system
> uses the page cache, but if it supports page cache writeback.
> E.g. for ramfs there is page cache, but it is the only data store.
> For shmemfs, there is writeback, but it is driven in a different
> way (through the anonymous memory code and not fs writeback).
> 
> And most importantly it just work, no need to mark file systems,
> as you're currently missing a lot of those that can have a fair
> number of inodes, e.g., sysfs, kernfs, cgroupfs or configfs.
> 
I ever added support for these file systems, but I deleted the code before
I sent out this patch.

To Jan Kara,
   thanks for the information about the issue.
   I will check the inode iteration issue firstly.

Thanks
Huang Shijie

[RFC PATCH v2 2/3] fs/super: introduce a helper sb_inodes_empty()

[Huang Shijie]

Introduce a helper sb_inodes_empty() which is used to
detect if the inodes list is empty.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
 fs/super.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/super.c b/fs/super.c
index 9d4025213521..e25ded0bc9a2 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -710,6 +710,11 @@ void retire_super(struct super_block *sb)
 }
 EXPORT_SYMBOL(retire_super);
 
+static bool sb_inodes_empty(struct super_block *sb)
+{
+	return list_empty(&sb->s_inodes);
+}
+
 /**
  *	generic_shutdown_super	-	common helper for ->kill_sb()
  *	@sb: superblock to kill
@@ -760,7 +765,7 @@ void generic_shutdown_super(struct super_block *sb)
 		 */
 		fscrypt_destroy_keyring(sb);
 
-		if (CHECK_DATA_CORRUPTION(!list_empty(&sb->s_inodes), NULL,
+		if (CHECK_DATA_CORRUPTION(!sb_inodes_empty(sb), NULL,
 				"VFS: Busy inodes after unmount of %s (%s)",
 				sb->s_id, sb->s_type->name)) {
 			/*
-- 
2.53.0

[RFC PATCH v2 3/3] fs/proc: split the inode list

[Huang Shijie]

The global s_inode_list_lock is heavily contended in procfs
on a 384-CPU, 12-NUMA-node Hygon machine running Hadoop TestDFSIO:
  #hadoop jar xxxx.jar TestDFSIO -read -nrFiles 1000 -size 100MB

The perf shows it consuming ~90% of the lock hotspot.
The lock is hit from both directions:
   -- inode creation (~49%) :
           getdents64 ->
              proc_readfd_common ->
	         new_inode ->
		   inode_sb_list_add()

   -- inode eviction (~41%)
           process exit ->
	      release_task ->
	         proc_invalidate_siblings_dcache ->
		     evict ->
		        inode_sb_list_del()

This patch spreads the inode list across per-shard locks for procfs:
   --- Add three fields in super_block:
        shards   : the pointer for the array of inode_shard.
        nr_shards: the size of the array
	s_inode_list_sharded: whether or not to use a sharded inode list

        struct inode_shard is cacheline-aligned to avoid false
        sharing between shard locks on different NUMA nodes.

   --- Add inode_list_add()/inode_list_del() callbacks to super_operations;
       procfs implements them to round-robin inodes
       onto nr_shards = DIV_ROUND_UP(num_possible_cpus(), 32)
       shards allocated at mount time, each protected by its own spinlock.

   --- For procfs, the "unmount" will call evict_inodes(),
       generic_shutdown_super() and hook_sb_delete() which will
       iterate the shards when the super_block inode list is sharded.
       Change these functions to work with the sharded inode list.

This reduces the s_inode_list_lock hotspot from ~90% to ~1% in TestDFSIO.
And we can improve the hadoop performance over 50%.

Signed-off-by: Huang Shijie <huangsj@hygon.cn>
---
 fs/inode.c                     |  79 ++++++++++------
 fs/proc/inode.c                |  53 +++++++++++
 fs/proc/internal.h             |   2 +
 fs/proc/root.c                 |   6 ++
 fs/super.c                     |  40 ++++++--
 include/linux/fs/super_types.h |  16 ++++
 include/linux/proc_fs.h        |   1 +
 security/landlock/fs.c         | 162 ++++++++++++++++++---------------
 8 files changed, 250 insertions(+), 109 deletions(-)

diff --git a/fs/inode.c b/fs/inode.c
index ba7da39be4a3..80acb9113487 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -635,6 +635,10 @@ void inode_sb_list_add(struct inode *inode)
 {
 	struct super_block *sb = inode->i_sb;
 
+	if (sb->s_inode_list_sharded && sb->s_op->inode_list_add) {
+		sb->s_op->inode_list_add(sb, inode);
+		return;
+	}
 	spin_lock(&sb->s_inode_list_lock);
 	list_add(&inode->i_sb_list, &sb->s_inodes);
 	spin_unlock(&sb->s_inode_list_lock);
@@ -646,6 +650,10 @@ static inline void inode_sb_list_del(struct inode *inode)
 	struct super_block *sb = inode->i_sb;
 
 	if (!list_empty(&inode->i_sb_list)) {
+		if (sb->s_inode_list_sharded && sb->s_op->inode_list_del) {
+			sb->s_op->inode_list_del(sb, inode);
+			return;
+		}
 		spin_lock(&sb->s_inode_list_lock);
 		list_del_init(&inode->i_sb_list);
 		spin_unlock(&sb->s_inode_list_lock);
@@ -879,41 +887,56 @@ void evict_inodes(struct super_block *sb)
 {
 	struct inode *inode;
 	LIST_HEAD(dispose);
+	struct list_head *head;
+	spinlock_t *lock;
+	unsigned int nr, i;
+	const bool sharded = sb->s_inode_list_sharded;
 
+	nr = sharded ? sb->nr_shards : 1;
 again:
-	spin_lock(&sb->s_inode_list_lock);
-	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
-		if (icount_read_once(inode))
-			continue;
-
-		spin_lock(&inode->i_lock);
-		if (icount_read(inode)) {
-			spin_unlock(&inode->i_lock);
-			continue;
-		}
-		if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE)) {
-			spin_unlock(&inode->i_lock);
-			continue;
+	for (i = 0; i < nr; i++) {
+		if (sharded) {
+			head = &sb->shards[i].list;
+			lock = &sb->shards[i].lock;
+		} else {
+			head = &sb->s_inodes;
+			lock = &sb->s_inode_list_lock;
 		}
 
-		inode_state_set(inode, I_FREEING);
-		inode_lru_list_del(inode);
-		spin_unlock(&inode->i_lock);
-		list_add(&inode->i_lru, &dispose);
+		spin_lock(lock);
+		list_for_each_entry(inode, head, i_sb_list) {
+			if (icount_read_once(inode))
+				continue;
 
-		/*
-		 * We can have a ton of inodes to evict at unmount time given
-		 * enough memory, check to see if we need to go to sleep for a
-		 * bit so we don't livelock.
-		 */
-		if (need_resched()) {
-			spin_unlock(&sb->s_inode_list_lock);
-			cond_resched();
-			dispose_list(&dispose);
-			goto again;
+			spin_lock(&inode->i_lock);
+			if (icount_read(inode)) {
+				spin_unlock(&inode->i_lock);
+				continue;
+			}
+			if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE)) {
+				spin_unlock(&inode->i_lock);
+				continue;
+			}
+
+			inode_state_set(inode, I_FREEING);
+			inode_lru_list_del(inode);
+			spin_unlock(&inode->i_lock);
+			list_add(&inode->i_lru, &dispose);
+
+			/*
+			 * We can have a ton of inodes to evict at unmount time given
+			 * enough memory, check to see if we need to go to sleep for a
+			 * bit so we don't livelock.
+			 */
+			if (need_resched()) {
+				spin_unlock(lock);
+				cond_resched();
+				dispose_list(&dispose);
+				goto again;
+			}
 		}
+		spin_unlock(lock);
 	}
-	spin_unlock(&sb->s_inode_list_lock);
 
 	dispose_list(&dispose);
 }
diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b7634f975d98..aeaf2e94e7ba 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -65,6 +65,7 @@ static struct inode *proc_alloc_inode(struct super_block *sb)
 	ei->sysctl_entry = NULL;
 	INIT_HLIST_NODE(&ei->sibling_inodes);
 	ei->ns_ops = NULL;
+	ei->shard_idx = 0;
 	return &ei->vfs_inode;
 }
 
@@ -80,6 +81,56 @@ static void proc_free_inode(struct inode *inode)
 	kmem_cache_free(proc_inode_cachep, PROC_I(inode));
 }
 
+static void proc_inode_list_add(struct super_block *sb, struct inode *inode)
+{
+	struct proc_fs_info *fs_info = proc_sb_info(sb);
+	struct proc_inode *ei = PROC_I(inode);
+	struct inode_shard *shard;
+	unsigned int idx, seq;
+
+	seq = atomic_fetch_inc(&fs_info->shard_seq);
+	idx = seq % sb->nr_shards;
+	ei->shard_idx = idx;
+	shard = &sb->shards[idx];
+
+	spin_lock(&shard->lock);
+	list_add(&inode->i_sb_list, &shard->list);
+	spin_unlock(&shard->lock);
+}
+
+static void proc_inode_list_del(struct super_block *sb, struct inode *inode)
+{
+	struct proc_inode *ei = PROC_I(inode);
+	struct inode_shard *shard = &sb->shards[ei->shard_idx];
+
+	spin_lock(&shard->lock);
+	list_del_init(&inode->i_sb_list);
+	spin_unlock(&shard->lock);
+}
+
+#define PROC_LIST_ALIGN	32
+int proc_init_inode_shards(struct super_block *sb)
+{
+	struct inode_shard *shards;
+	unsigned int nr;
+	int i;
+
+	nr = DIV_ROUND_UP(num_possible_cpus(), PROC_LIST_ALIGN);
+	shards = kcalloc(nr, sizeof(*shards), GFP_KERNEL);
+	if (!shards)
+		return -ENOMEM;
+
+	for (i = 0; i < nr; i++) {
+		INIT_LIST_HEAD(&shards[i].list);
+		spin_lock_init(&shards[i].lock);
+	}
+
+	sb->shards = shards;
+	sb->nr_shards = nr;
+	sb->s_inode_list_sharded = true;
+	return 0;
+}
+
 static void init_once(void *foo)
 {
 	struct proc_inode *ei = (struct proc_inode *) foo;
@@ -191,6 +242,8 @@ const struct super_operations proc_sops = {
 	.evict_inode	= proc_evict_inode,
 	.statfs		= simple_statfs,
 	.show_options	= proc_show_options,
+	.inode_list_add	= proc_inode_list_add,
+	.inode_list_del	= proc_inode_list_del,
 };
 
 enum {BIAS = -1U<<31};
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 04bd6c9e65a7..edbe357263c2 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -127,6 +127,7 @@ struct proc_inode {
 	struct hlist_node sibling_inodes;
 	const struct proc_ns_operations *ns_ops;
 	struct inode vfs_inode;
+	unsigned int shard_idx;
 } __randomize_layout;
 
 /*
@@ -317,6 +318,7 @@ void proc_init_kmemcache(void);
 void proc_invalidate_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock);
 void set_proc_pid_nlink(void);
 extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
+extern int proc_init_inode_shards(struct super_block *);
 extern void proc_entry_rundown(struct proc_dir_entry *);
 
 /*
diff --git a/fs/proc/root.c b/fs/proc/root.c
index d45f5af5ab53..fb5d68da85f4 100644
--- a/fs/proc/root.c
+++ b/fs/proc/root.c
@@ -275,6 +275,10 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
 	s->s_time_gran = 1;
 	s->s_fs_info = fs_info;
 
+	ret = proc_init_inode_shards(s);
+	if (ret)
+		return ret;
+
 	if (fs_info->pidonly == PROC_PIDONLY_ON)
 		s->s_iflags |= SB_I_RESTRICTED_VARIANT;
 
@@ -359,6 +363,8 @@ static void proc_kill_sb(struct super_block *sb)
 	struct proc_fs_info *fs_info = proc_sb_info(sb);
 
 	kill_anon_super(sb);
+	if (sb->s_inode_list_sharded)
+		kfree(sb->shards);
 	if (fs_info) {
 		put_pid_ns(fs_info->pid_ns);
 		put_cred(fs_info->mounter_cred);
diff --git a/fs/super.c b/fs/super.c
index e25ded0bc9a2..e44db4505238 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -712,7 +712,16 @@ EXPORT_SYMBOL(retire_super);
 
 static bool sb_inodes_empty(struct super_block *sb)
 {
-	return list_empty(&sb->s_inodes);
+	unsigned int i;
+
+	if (!sb->s_inode_list_sharded)
+		return list_empty(&sb->s_inodes);
+
+	for (i = 0; i < sb->nr_shards; i++)
+		if (!list_empty(&sb->shards[i].list))
+			return false;
+
+	return true;
 }
 
 /**
@@ -774,14 +783,29 @@ void generic_shutdown_super(struct super_block *sb)
 			 * iput_final() or such crashes cleanly.
 			 */
 			struct inode *inode;
-
-			spin_lock(&sb->s_inode_list_lock);
-			list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
-				inode->i_op = VFS_PTR_POISON;
-				inode->i_sb = VFS_PTR_POISON;
-				inode->i_mapping = VFS_PTR_POISON;
+			struct list_head *head;
+			spinlock_t *lock;
+			unsigned int nr, i;
+			const bool sharded = sb->s_inode_list_sharded;
+
+			nr = sharded ? sb->nr_shards : 1;
+			for (i = 0; i < nr; i++) {
+				if (sharded) {
+					head = &sb->shards[i].list;
+					lock = &sb->shards[i].lock;
+				} else {
+					head = &sb->s_inodes;
+					lock = &sb->s_inode_list_lock;
+				}
+
+				spin_lock(lock);
+				list_for_each_entry(inode, head, i_sb_list) {
+					inode->i_op = VFS_PTR_POISON;
+					inode->i_sb = VFS_PTR_POISON;
+					inode->i_mapping = VFS_PTR_POISON;
+				}
+				spin_unlock(lock);
 			}
-			spin_unlock(&sb->s_inode_list_lock);
 		}
 	}
 	/*
diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
index 1a596caf58a8..f06f293b46be 100644
--- a/include/linux/fs/super_types.h
+++ b/include/linux/fs/super_types.h
@@ -130,8 +130,20 @@ struct super_operations {
 
 	/* Report a filesystem error */
 	void (*report_error)(const struct fserror_event *event);
+
+	void (*inode_list_add)(struct super_block *sb, struct inode *inode);
+	void (*inode_list_del)(struct super_block *sb, struct inode *inode);
 };
 
+/*
+ * Sharded inode list which is used to spread s_inode_list_lock contention
+ * across per-shard locks.
+ */
+struct inode_shard {
+	struct list_head	list;
+	spinlock_t		lock;
+} ____cacheline_aligned_in_smp;
+
 struct super_block {
 	struct list_head			s_list;		/* Keep this first */
 	dev_t					s_dev;		/* search index; _not_ kdev_t */
@@ -269,6 +281,10 @@ struct super_block {
 	 */
 	int s_stack_depth;
 
+	bool					s_inode_list_sharded;
+	struct inode_shard			*shards;
+	unsigned int				nr_shards;
+
 	/* s_inode_list_lock protects s_inodes */
 	spinlock_t				s_inode_list_lock ____cacheline_aligned_in_smp;
 	struct list_head			s_inodes;	/* all inodes */
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index 47d7deaeed8f..37eed34f3a93 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -70,6 +70,7 @@ struct proc_fs_info {
 	const struct cred *mounter_cred;
 	enum proc_hidepid hide_pid;
 	enum proc_pidonly pidonly;
+	atomic_t shard_seq;
 	struct rcu_head rcu;
 };
 
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 330a1871bf94..8360a6a8a3ad 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1383,98 +1383,114 @@ static void hook_inode_free_security_rcu(void *inode_security)
 static void hook_sb_delete(struct super_block *const sb)
 {
 	struct inode *inode, *prev_inode = NULL;
+	unsigned int nr, i;
 
 	if (!landlock_initialized)
 		return;
 
-	spin_lock(&sb->s_inode_list_lock);
-	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
-		struct landlock_object *object;
+	nr = sb->s_inode_list_sharded ? sb->nr_shards : 1;
 
-		/* Only handles referenced inodes. */
-		if (!icount_read_once(inode))
-			continue;
+	for (i = 0; i < nr; i++) {
+		struct list_head *head;
+		spinlock_t *lock;
 
-		/*
-		 * Protects against concurrent modification of inode (e.g.
-		 * from get_inode_object()).
-		 */
-		spin_lock(&inode->i_lock);
-		/*
-		 * Checks I_FREEING and I_WILL_FREE  to protect against a race
-		 * condition when release_inode() just called iput(), which
-		 * could lead to a NULL dereference of inode->security or a
-		 * second call to iput() for the same Landlock object.  Also
-		 * checks I_NEW because such inode cannot be tied to an object.
-		 */
-		if (inode_state_read(inode) &
-		    (I_FREEING | I_WILL_FREE | I_NEW)) {
-			spin_unlock(&inode->i_lock);
-			continue;
+		if (sb->s_inode_list_sharded) {
+			head = &sb->shards[i].list;
+			lock = &sb->shards[i].lock;
+		} else {
+			head = &sb->s_inodes;
+			lock = &sb->s_inode_list_lock;
 		}
 
-		rcu_read_lock();
-		object = rcu_dereference(landlock_inode(inode)->object);
-		if (!object) {
-			rcu_read_unlock();
-			spin_unlock(&inode->i_lock);
-			continue;
-		}
-		/* Keeps a reference to this inode until the next loop walk. */
-		__iget(inode);
-		spin_unlock(&inode->i_lock);
+		spin_lock(lock);
+		list_for_each_entry(inode, head, i_sb_list) {
+			struct landlock_object *object;
 
-		/*
-		 * If there is no concurrent release_inode() ongoing, then we
-		 * are in charge of calling iput() on this inode, otherwise we
-		 * will just wait for it to finish.
-		 */
-		spin_lock(&object->lock);
-		if (object->underobj == inode) {
-			object->underobj = NULL;
-			spin_unlock(&object->lock);
-			rcu_read_unlock();
+			/* Only handles referenced inodes. */
+			if (!icount_read_once(inode))
+				continue;
 
 			/*
-			 * Because object->underobj was not NULL,
-			 * release_inode() and get_inode_object() guarantee
-			 * that it is safe to reset
-			 * landlock_inode(inode)->object while it is not NULL.
-			 * It is therefore not necessary to lock inode->i_lock.
+			 * Protects against concurrent modification of inode (e.g.
+			 * from get_inode_object()).
 			 */
-			rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+			spin_lock(&inode->i_lock);
 			/*
-			 * At this point, we own the ihold() reference that was
-			 * originally set up by get_inode_object() and the
-			 * __iget() reference that we just set in this loop
-			 * walk.  Therefore there are at least two references
-			 * on the inode.
+			 * Checks I_FREEING and I_WILL_FREE  to protect against a race
+			 * condition when release_inode() just called iput(), which
+			 * could lead to a NULL dereference of inode->security or a
+			 * second call to iput() for the same Landlock object.  Also
+			 * checks I_NEW because such inode cannot be tied to an object.
 			 */
-			iput_not_last(inode);
-		} else {
-			spin_unlock(&object->lock);
-			rcu_read_unlock();
-		}
+			if (inode_state_read(inode) &
+			    (I_FREEING | I_WILL_FREE | I_NEW)) {
+				spin_unlock(&inode->i_lock);
+				continue;
+			}
+
+			rcu_read_lock();
+			object = rcu_dereference(landlock_inode(inode)->object);
+			if (!object) {
+				rcu_read_unlock();
+				spin_unlock(&inode->i_lock);
+				continue;
+			}
+			/* Keeps a reference to this inode until the next loop walk. */
+			__iget(inode);
+			spin_unlock(&inode->i_lock);
 
-		if (prev_inode) {
-			/*
-			 * At this point, we still own the __iget() reference
-			 * that we just set in this loop walk.  Therefore we
-			 * can drop the list lock and know that the inode won't
-			 * disappear from under us until the next loop walk.
-			 */
-			spin_unlock(&sb->s_inode_list_lock);
 			/*
-			 * We can now actually put the inode reference from the
-			 * previous loop walk, which is not needed anymore.
+			 * If there is no concurrent release_inode() ongoing, then we
+			 * are in charge of calling iput() on this inode, otherwise we
+			 * will just wait for it to finish.
 			 */
-			iput(prev_inode);
-			cond_resched();
-			spin_lock(&sb->s_inode_list_lock);
+			spin_lock(&object->lock);
+			if (object->underobj == inode) {
+				object->underobj = NULL;
+				spin_unlock(&object->lock);
+				rcu_read_unlock();
+
+				/*
+				 * Because object->underobj was not NULL,
+				 * release_inode() and get_inode_object() guarantee
+				 * that it is safe to reset
+				 * landlock_inode(inode)->object while it is not NULL.
+				 * It is therefore not necessary to lock inode->i_lock.
+				 */
+				rcu_assign_pointer(landlock_inode(inode)->object, NULL);
+				/*
+				 * At this point, we own the ihold() reference that was
+				 * originally set up by get_inode_object() and the
+				 * __iget() reference that we just set in this loop
+				 * walk.  Therefore there are at least two references
+				 * on the inode.
+				 */
+				iput_not_last(inode);
+			} else {
+				spin_unlock(&object->lock);
+				rcu_read_unlock();
+			}
+
+			if (prev_inode) {
+				/*
+				 * At this point, we still own the __iget() reference
+				 * that we just set in this loop walk.  Therefore we
+				 * can drop the list lock and know that the inode won't
+				 * disappear from under us until the next loop walk.
+				 */
+				spin_unlock(lock);
+				/*
+				 * We can now actually put the inode reference from the
+				 * previous loop walk, which is not needed anymore.
+				 */
+				iput(prev_inode);
+				cond_resched();
+				spin_lock(lock);
+			}
+			prev_inode = inode;
 		}
-		prev_inode = inode;
+		spin_unlock(lock);
 	}
-	spin_unlock(&sb->s_inode_list_lock);
 
 	/* Puts the inode reference from the last loop walk, if any. */
 	if (prev_inode)
-- 
2.53.0

Re: [RFC PATCH v2 3/3] fs/proc: split the inode list

[Jan Kara]

On Sun 20-09-26 15:28:11, Huang Shijie wrote:
> The global s_inode_list_lock is heavily contended in procfs
> on a 384-CPU, 12-NUMA-node Hygon machine running Hadoop TestDFSIO:
>   #hadoop jar xxxx.jar TestDFSIO -read -nrFiles 1000 -size 100MB
> 
> The perf shows it consuming ~90% of the lock hotspot.
> The lock is hit from both directions:
>    -- inode creation (~49%) :
>            getdents64 ->
>               proc_readfd_common ->
> 	         new_inode ->
> 		   inode_sb_list_add()
> 
>    -- inode eviction (~41%)
>            process exit ->
> 	      release_task ->
> 	         proc_invalidate_siblings_dcache ->
> 		     evict ->
> 		        inode_sb_list_del()
> 
> This patch spreads the inode list across per-shard locks for procfs:
>    --- Add three fields in super_block:
>         shards   : the pointer for the array of inode_shard.
>         nr_shards: the size of the array
> 	s_inode_list_sharded: whether or not to use a sharded inode list
> 
>         struct inode_shard is cacheline-aligned to avoid false
>         sharing between shard locks on different NUMA nodes.
> 
>    --- Add inode_list_add()/inode_list_del() callbacks to super_operations;
>        procfs implements them to round-robin inodes
>        onto nr_shards = DIV_ROUND_UP(num_possible_cpus(), 32)
>        shards allocated at mount time, each protected by its own spinlock.
> 
>    --- For procfs, the "unmount" will call evict_inodes(),
>        generic_shutdown_super() and hook_sb_delete() which will
>        iterate the shards when the super_block inode list is sharded.
>        Change these functions to work with the sharded inode list.
> 
> This reduces the s_inode_list_lock hotspot from ~90% to ~1% in TestDFSIO.
> And we can improve the hadoop performance over 50%.
> 
> Signed-off-by: Huang Shijie <huangsj@hygon.cn>

It has been recognized in the past that superblock global inode list is a
scalability bottleneck for certain loads. Not only for procfs but even for
XFS or other disk-based filesystems. I think last this was discussed here
[1]. So I'm not if favor of trying to deal with it just for procfs, that's
just ugly and will backfire pretty quickly.

Of course more generic solution needs more work, in particular we first
need to abstract out inode iteration. Julian might be already looking into
that [2] so better talk to him before investing significant time into that.
After inode iteration is abstracted, we can relatively painlessly switch
the data structure tracking inodes per superblock to something more
scalable.

								Honza

[1] https://lore.kernel.org/all/20241002014017.3801899-1-david@fromorbit.com/
[2] https://lore.kernel.org/all/1891f4ac-139e-4e9c-80ff-802ec2bb2844@bytedance.com/


> ---
>  fs/inode.c                     |  79 ++++++++++------
>  fs/proc/inode.c                |  53 +++++++++++
>  fs/proc/internal.h             |   2 +
>  fs/proc/root.c                 |   6 ++
>  fs/super.c                     |  40 ++++++--
>  include/linux/fs/super_types.h |  16 ++++
>  include/linux/proc_fs.h        |   1 +
>  security/landlock/fs.c         | 162 ++++++++++++++++++---------------
>  8 files changed, 250 insertions(+), 109 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index ba7da39be4a3..80acb9113487 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -635,6 +635,10 @@ void inode_sb_list_add(struct inode *inode)
>  {
>  	struct super_block *sb = inode->i_sb;
>  
> +	if (sb->s_inode_list_sharded && sb->s_op->inode_list_add) {
> +		sb->s_op->inode_list_add(sb, inode);
> +		return;
> +	}
>  	spin_lock(&sb->s_inode_list_lock);
>  	list_add(&inode->i_sb_list, &sb->s_inodes);
>  	spin_unlock(&sb->s_inode_list_lock);
> @@ -646,6 +650,10 @@ static inline void inode_sb_list_del(struct inode *inode)
>  	struct super_block *sb = inode->i_sb;
>  
>  	if (!list_empty(&inode->i_sb_list)) {
> +		if (sb->s_inode_list_sharded && sb->s_op->inode_list_del) {
> +			sb->s_op->inode_list_del(sb, inode);
> +			return;
> +		}
>  		spin_lock(&sb->s_inode_list_lock);
>  		list_del_init(&inode->i_sb_list);
>  		spin_unlock(&sb->s_inode_list_lock);
> @@ -879,41 +887,56 @@ void evict_inodes(struct super_block *sb)
>  {
>  	struct inode *inode;
>  	LIST_HEAD(dispose);
> +	struct list_head *head;
> +	spinlock_t *lock;
> +	unsigned int nr, i;
> +	const bool sharded = sb->s_inode_list_sharded;
>  
> +	nr = sharded ? sb->nr_shards : 1;
>  again:
> -	spin_lock(&sb->s_inode_list_lock);
> -	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> -		if (icount_read_once(inode))
> -			continue;
> -
> -		spin_lock(&inode->i_lock);
> -		if (icount_read(inode)) {
> -			spin_unlock(&inode->i_lock);
> -			continue;
> -		}
> -		if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE)) {
> -			spin_unlock(&inode->i_lock);
> -			continue;
> +	for (i = 0; i < nr; i++) {
> +		if (sharded) {
> +			head = &sb->shards[i].list;
> +			lock = &sb->shards[i].lock;
> +		} else {
> +			head = &sb->s_inodes;
> +			lock = &sb->s_inode_list_lock;
>  		}
>  
> -		inode_state_set(inode, I_FREEING);
> -		inode_lru_list_del(inode);
> -		spin_unlock(&inode->i_lock);
> -		list_add(&inode->i_lru, &dispose);
> +		spin_lock(lock);
> +		list_for_each_entry(inode, head, i_sb_list) {
> +			if (icount_read_once(inode))
> +				continue;
>  
> -		/*
> -		 * We can have a ton of inodes to evict at unmount time given
> -		 * enough memory, check to see if we need to go to sleep for a
> -		 * bit so we don't livelock.
> -		 */
> -		if (need_resched()) {
> -			spin_unlock(&sb->s_inode_list_lock);
> -			cond_resched();
> -			dispose_list(&dispose);
> -			goto again;
> +			spin_lock(&inode->i_lock);
> +			if (icount_read(inode)) {
> +				spin_unlock(&inode->i_lock);
> +				continue;
> +			}
> +			if (inode_state_read(inode) & (I_NEW | I_FREEING | I_WILL_FREE)) {
> +				spin_unlock(&inode->i_lock);
> +				continue;
> +			}
> +
> +			inode_state_set(inode, I_FREEING);
> +			inode_lru_list_del(inode);
> +			spin_unlock(&inode->i_lock);
> +			list_add(&inode->i_lru, &dispose);
> +
> +			/*
> +			 * We can have a ton of inodes to evict at unmount time given
> +			 * enough memory, check to see if we need to go to sleep for a
> +			 * bit so we don't livelock.
> +			 */
> +			if (need_resched()) {
> +				spin_unlock(lock);
> +				cond_resched();
> +				dispose_list(&dispose);
> +				goto again;
> +			}
>  		}
> +		spin_unlock(lock);
>  	}
> -	spin_unlock(&sb->s_inode_list_lock);
>  
>  	dispose_list(&dispose);
>  }
> diff --git a/fs/proc/inode.c b/fs/proc/inode.c
> index b7634f975d98..aeaf2e94e7ba 100644
> --- a/fs/proc/inode.c
> +++ b/fs/proc/inode.c
> @@ -65,6 +65,7 @@ static struct inode *proc_alloc_inode(struct super_block *sb)
>  	ei->sysctl_entry = NULL;
>  	INIT_HLIST_NODE(&ei->sibling_inodes);
>  	ei->ns_ops = NULL;
> +	ei->shard_idx = 0;
>  	return &ei->vfs_inode;
>  }
>  
> @@ -80,6 +81,56 @@ static void proc_free_inode(struct inode *inode)
>  	kmem_cache_free(proc_inode_cachep, PROC_I(inode));
>  }
>  
> +static void proc_inode_list_add(struct super_block *sb, struct inode *inode)
> +{
> +	struct proc_fs_info *fs_info = proc_sb_info(sb);
> +	struct proc_inode *ei = PROC_I(inode);
> +	struct inode_shard *shard;
> +	unsigned int idx, seq;
> +
> +	seq = atomic_fetch_inc(&fs_info->shard_seq);
> +	idx = seq % sb->nr_shards;
> +	ei->shard_idx = idx;
> +	shard = &sb->shards[idx];
> +
> +	spin_lock(&shard->lock);
> +	list_add(&inode->i_sb_list, &shard->list);
> +	spin_unlock(&shard->lock);
> +}
> +
> +static void proc_inode_list_del(struct super_block *sb, struct inode *inode)
> +{
> +	struct proc_inode *ei = PROC_I(inode);
> +	struct inode_shard *shard = &sb->shards[ei->shard_idx];
> +
> +	spin_lock(&shard->lock);
> +	list_del_init(&inode->i_sb_list);
> +	spin_unlock(&shard->lock);
> +}
> +
> +#define PROC_LIST_ALIGN	32
> +int proc_init_inode_shards(struct super_block *sb)
> +{
> +	struct inode_shard *shards;
> +	unsigned int nr;
> +	int i;
> +
> +	nr = DIV_ROUND_UP(num_possible_cpus(), PROC_LIST_ALIGN);
> +	shards = kcalloc(nr, sizeof(*shards), GFP_KERNEL);
> +	if (!shards)
> +		return -ENOMEM;
> +
> +	for (i = 0; i < nr; i++) {
> +		INIT_LIST_HEAD(&shards[i].list);
> +		spin_lock_init(&shards[i].lock);
> +	}
> +
> +	sb->shards = shards;
> +	sb->nr_shards = nr;
> +	sb->s_inode_list_sharded = true;
> +	return 0;
> +}
> +
>  static void init_once(void *foo)
>  {
>  	struct proc_inode *ei = (struct proc_inode *) foo;
> @@ -191,6 +242,8 @@ const struct super_operations proc_sops = {
>  	.evict_inode	= proc_evict_inode,
>  	.statfs		= simple_statfs,
>  	.show_options	= proc_show_options,
> +	.inode_list_add	= proc_inode_list_add,
> +	.inode_list_del	= proc_inode_list_del,
>  };
>  
>  enum {BIAS = -1U<<31};
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 04bd6c9e65a7..edbe357263c2 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -127,6 +127,7 @@ struct proc_inode {
>  	struct hlist_node sibling_inodes;
>  	const struct proc_ns_operations *ns_ops;
>  	struct inode vfs_inode;
> +	unsigned int shard_idx;
>  } __randomize_layout;
>  
>  /*
> @@ -317,6 +318,7 @@ void proc_init_kmemcache(void);
>  void proc_invalidate_siblings_dcache(struct hlist_head *inodes, spinlock_t *lock);
>  void set_proc_pid_nlink(void);
>  extern struct inode *proc_get_inode(struct super_block *, struct proc_dir_entry *);
> +extern int proc_init_inode_shards(struct super_block *);
>  extern void proc_entry_rundown(struct proc_dir_entry *);
>  
>  /*
> diff --git a/fs/proc/root.c b/fs/proc/root.c
> index d45f5af5ab53..fb5d68da85f4 100644
> --- a/fs/proc/root.c
> +++ b/fs/proc/root.c
> @@ -275,6 +275,10 @@ static int proc_fill_super(struct super_block *s, struct fs_context *fc)
>  	s->s_time_gran = 1;
>  	s->s_fs_info = fs_info;
>  
> +	ret = proc_init_inode_shards(s);
> +	if (ret)
> +		return ret;
> +
>  	if (fs_info->pidonly == PROC_PIDONLY_ON)
>  		s->s_iflags |= SB_I_RESTRICTED_VARIANT;
>  
> @@ -359,6 +363,8 @@ static void proc_kill_sb(struct super_block *sb)
>  	struct proc_fs_info *fs_info = proc_sb_info(sb);
>  
>  	kill_anon_super(sb);
> +	if (sb->s_inode_list_sharded)
> +		kfree(sb->shards);
>  	if (fs_info) {
>  		put_pid_ns(fs_info->pid_ns);
>  		put_cred(fs_info->mounter_cred);
> diff --git a/fs/super.c b/fs/super.c
> index e25ded0bc9a2..e44db4505238 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -712,7 +712,16 @@ EXPORT_SYMBOL(retire_super);
>  
>  static bool sb_inodes_empty(struct super_block *sb)
>  {
> -	return list_empty(&sb->s_inodes);
> +	unsigned int i;
> +
> +	if (!sb->s_inode_list_sharded)
> +		return list_empty(&sb->s_inodes);
> +
> +	for (i = 0; i < sb->nr_shards; i++)
> +		if (!list_empty(&sb->shards[i].list))
> +			return false;
> +
> +	return true;
>  }
>  
>  /**
> @@ -774,14 +783,29 @@ void generic_shutdown_super(struct super_block *sb)
>  			 * iput_final() or such crashes cleanly.
>  			 */
>  			struct inode *inode;
> -
> -			spin_lock(&sb->s_inode_list_lock);
> -			list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> -				inode->i_op = VFS_PTR_POISON;
> -				inode->i_sb = VFS_PTR_POISON;
> -				inode->i_mapping = VFS_PTR_POISON;
> +			struct list_head *head;
> +			spinlock_t *lock;
> +			unsigned int nr, i;
> +			const bool sharded = sb->s_inode_list_sharded;
> +
> +			nr = sharded ? sb->nr_shards : 1;
> +			for (i = 0; i < nr; i++) {
> +				if (sharded) {
> +					head = &sb->shards[i].list;
> +					lock = &sb->shards[i].lock;
> +				} else {
> +					head = &sb->s_inodes;
> +					lock = &sb->s_inode_list_lock;
> +				}
> +
> +				spin_lock(lock);
> +				list_for_each_entry(inode, head, i_sb_list) {
> +					inode->i_op = VFS_PTR_POISON;
> +					inode->i_sb = VFS_PTR_POISON;
> +					inode->i_mapping = VFS_PTR_POISON;
> +				}
> +				spin_unlock(lock);
>  			}
> -			spin_unlock(&sb->s_inode_list_lock);
>  		}
>  	}
>  	/*
> diff --git a/include/linux/fs/super_types.h b/include/linux/fs/super_types.h
> index 1a596caf58a8..f06f293b46be 100644
> --- a/include/linux/fs/super_types.h
> +++ b/include/linux/fs/super_types.h
> @@ -130,8 +130,20 @@ struct super_operations {
>  
>  	/* Report a filesystem error */
>  	void (*report_error)(const struct fserror_event *event);
> +
> +	void (*inode_list_add)(struct super_block *sb, struct inode *inode);
> +	void (*inode_list_del)(struct super_block *sb, struct inode *inode);
>  };
>  
> +/*
> + * Sharded inode list which is used to spread s_inode_list_lock contention
> + * across per-shard locks.
> + */
> +struct inode_shard {
> +	struct list_head	list;
> +	spinlock_t		lock;
> +} ____cacheline_aligned_in_smp;
> +
>  struct super_block {
>  	struct list_head			s_list;		/* Keep this first */
>  	dev_t					s_dev;		/* search index; _not_ kdev_t */
> @@ -269,6 +281,10 @@ struct super_block {
>  	 */
>  	int s_stack_depth;
>  
> +	bool					s_inode_list_sharded;
> +	struct inode_shard			*shards;
> +	unsigned int				nr_shards;
> +
>  	/* s_inode_list_lock protects s_inodes */
>  	spinlock_t				s_inode_list_lock ____cacheline_aligned_in_smp;
>  	struct list_head			s_inodes;	/* all inodes */
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index 47d7deaeed8f..37eed34f3a93 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -70,6 +70,7 @@ struct proc_fs_info {
>  	const struct cred *mounter_cred;
>  	enum proc_hidepid hide_pid;
>  	enum proc_pidonly pidonly;
> +	atomic_t shard_seq;
>  	struct rcu_head rcu;
>  };
>  
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 330a1871bf94..8360a6a8a3ad 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1383,98 +1383,114 @@ static void hook_inode_free_security_rcu(void *inode_security)
>  static void hook_sb_delete(struct super_block *const sb)
>  {
>  	struct inode *inode, *prev_inode = NULL;
> +	unsigned int nr, i;
>  
>  	if (!landlock_initialized)
>  		return;
>  
> -	spin_lock(&sb->s_inode_list_lock);
> -	list_for_each_entry(inode, &sb->s_inodes, i_sb_list) {
> -		struct landlock_object *object;
> +	nr = sb->s_inode_list_sharded ? sb->nr_shards : 1;
>  
> -		/* Only handles referenced inodes. */
> -		if (!icount_read_once(inode))
> -			continue;
> +	for (i = 0; i < nr; i++) {
> +		struct list_head *head;
> +		spinlock_t *lock;
>  
> -		/*
> -		 * Protects against concurrent modification of inode (e.g.
> -		 * from get_inode_object()).
> -		 */
> -		spin_lock(&inode->i_lock);
> -		/*
> -		 * Checks I_FREEING and I_WILL_FREE  to protect against a race
> -		 * condition when release_inode() just called iput(), which
> -		 * could lead to a NULL dereference of inode->security or a
> -		 * second call to iput() for the same Landlock object.  Also
> -		 * checks I_NEW because such inode cannot be tied to an object.
> -		 */
> -		if (inode_state_read(inode) &
> -		    (I_FREEING | I_WILL_FREE | I_NEW)) {
> -			spin_unlock(&inode->i_lock);
> -			continue;
> +		if (sb->s_inode_list_sharded) {
> +			head = &sb->shards[i].list;
> +			lock = &sb->shards[i].lock;
> +		} else {
> +			head = &sb->s_inodes;
> +			lock = &sb->s_inode_list_lock;
>  		}
>  
> -		rcu_read_lock();
> -		object = rcu_dereference(landlock_inode(inode)->object);
> -		if (!object) {
> -			rcu_read_unlock();
> -			spin_unlock(&inode->i_lock);
> -			continue;
> -		}
> -		/* Keeps a reference to this inode until the next loop walk. */
> -		__iget(inode);
> -		spin_unlock(&inode->i_lock);
> +		spin_lock(lock);
> +		list_for_each_entry(inode, head, i_sb_list) {
> +			struct landlock_object *object;
>  
> -		/*
> -		 * If there is no concurrent release_inode() ongoing, then we
> -		 * are in charge of calling iput() on this inode, otherwise we
> -		 * will just wait for it to finish.
> -		 */
> -		spin_lock(&object->lock);
> -		if (object->underobj == inode) {
> -			object->underobj = NULL;
> -			spin_unlock(&object->lock);
> -			rcu_read_unlock();
> +			/* Only handles referenced inodes. */
> +			if (!icount_read_once(inode))
> +				continue;
>  
>  			/*
> -			 * Because object->underobj was not NULL,
> -			 * release_inode() and get_inode_object() guarantee
> -			 * that it is safe to reset
> -			 * landlock_inode(inode)->object while it is not NULL.
> -			 * It is therefore not necessary to lock inode->i_lock.
> +			 * Protects against concurrent modification of inode (e.g.
> +			 * from get_inode_object()).
>  			 */
> -			rcu_assign_pointer(landlock_inode(inode)->object, NULL);
> +			spin_lock(&inode->i_lock);
>  			/*
> -			 * At this point, we own the ihold() reference that was
> -			 * originally set up by get_inode_object() and the
> -			 * __iget() reference that we just set in this loop
> -			 * walk.  Therefore there are at least two references
> -			 * on the inode.
> +			 * Checks I_FREEING and I_WILL_FREE  to protect against a race
> +			 * condition when release_inode() just called iput(), which
> +			 * could lead to a NULL dereference of inode->security or a
> +			 * second call to iput() for the same Landlock object.  Also
> +			 * checks I_NEW because such inode cannot be tied to an object.
>  			 */
> -			iput_not_last(inode);
> -		} else {
> -			spin_unlock(&object->lock);
> -			rcu_read_unlock();
> -		}
> +			if (inode_state_read(inode) &
> +			    (I_FREEING | I_WILL_FREE | I_NEW)) {
> +				spin_unlock(&inode->i_lock);
> +				continue;
> +			}
> +
> +			rcu_read_lock();
> +			object = rcu_dereference(landlock_inode(inode)->object);
> +			if (!object) {
> +				rcu_read_unlock();
> +				spin_unlock(&inode->i_lock);
> +				continue;
> +			}
> +			/* Keeps a reference to this inode until the next loop walk. */
> +			__iget(inode);
> +			spin_unlock(&inode->i_lock);
>  
> -		if (prev_inode) {
> -			/*
> -			 * At this point, we still own the __iget() reference
> -			 * that we just set in this loop walk.  Therefore we
> -			 * can drop the list lock and know that the inode won't
> -			 * disappear from under us until the next loop walk.
> -			 */
> -			spin_unlock(&sb->s_inode_list_lock);
>  			/*
> -			 * We can now actually put the inode reference from the
> -			 * previous loop walk, which is not needed anymore.
> +			 * If there is no concurrent release_inode() ongoing, then we
> +			 * are in charge of calling iput() on this inode, otherwise we
> +			 * will just wait for it to finish.
>  			 */
> -			iput(prev_inode);
> -			cond_resched();
> -			spin_lock(&sb->s_inode_list_lock);
> +			spin_lock(&object->lock);
> +			if (object->underobj == inode) {
> +				object->underobj = NULL;
> +				spin_unlock(&object->lock);
> +				rcu_read_unlock();
> +
> +				/*
> +				 * Because object->underobj was not NULL,
> +				 * release_inode() and get_inode_object() guarantee
> +				 * that it is safe to reset
> +				 * landlock_inode(inode)->object while it is not NULL.
> +				 * It is therefore not necessary to lock inode->i_lock.
> +				 */
> +				rcu_assign_pointer(landlock_inode(inode)->object, NULL);
> +				/*
> +				 * At this point, we own the ihold() reference that was
> +				 * originally set up by get_inode_object() and the
> +				 * __iget() reference that we just set in this loop
> +				 * walk.  Therefore there are at least two references
> +				 * on the inode.
> +				 */
> +				iput_not_last(inode);
> +			} else {
> +				spin_unlock(&object->lock);
> +				rcu_read_unlock();
> +			}
> +
> +			if (prev_inode) {
> +				/*
> +				 * At this point, we still own the __iget() reference
> +				 * that we just set in this loop walk.  Therefore we
> +				 * can drop the list lock and know that the inode won't
> +				 * disappear from under us until the next loop walk.
> +				 */
> +				spin_unlock(lock);
> +				/*
> +				 * We can now actually put the inode reference from the
> +				 * previous loop walk, which is not needed anymore.
> +				 */
> +				iput(prev_inode);
> +				cond_resched();
> +				spin_lock(lock);
> +			}
> +			prev_inode = inode;
>  		}
> -		prev_inode = inode;
> +		spin_unlock(lock);
>  	}
> -	spin_unlock(&sb->s_inode_list_lock);
>  
>  	/* Puts the inode reference from the last loop walk, if any. */
>  	if (prev_inode)
> -- 
> 2.53.0
> 
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR