linux-kernel - Re: [PATCH kernfs 1/3] kernfs: switch global kernfs_idr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250412115054.25580-1-alexjlzheng@tencent.com>
Date: Sat, 12 Apr 2025 19:50:54 +0800
From: alexjlzheng@...il.com
To: gregkh@...uxfoundation.org
Cc: alexjlzheng@...il.com,
	alexjlzheng@...cent.com,
	linux-kernel@...r.kernel.org,
	tj@...nel.org
Subject: Re: [PATCH kernfs 1/3] kernfs: switch global kernfs_idr_lock to per-fs lock

On Sat, 12 Apr 2025 08:12:22 +0200, gregkh@...uxfoundation.org wrote:
> On Sat, Apr 13, 2025 at 02:31:07AM +0800, alexjlzheng@...il.com wrote:
> > From: Jinliang Zheng <alexjlzheng@...cent.com>
> > 
> > The kernfs implementation has big lock granularity(kernfs_idr_lock) so
> > every kernfs-based(e.g., sysfs, cgroup) fs are able to compete the lock.
> > 
> > This patch switches the global kernfs_idr_lock to per-fs lock, which
> > put the spinlock into kernfs_root.
> > 
> > Signed-off-by: Jinliang Zheng <alexjlzheng@...cent.com>
> > ---
> >  fs/kernfs/dir.c             | 14 +++++++-------
> >  fs/kernfs/kernfs-internal.h |  1 +
> >  2 files changed, 8 insertions(+), 7 deletions(-)
> 
> What kind of testing / benchmark did you do for this series that shows
> that this works, AND that this actually is measureable?  What workload
> are you doing that causes these changes to be needed?

Thank you for your reply. :)

We are trying to implement a kernfs-based filesystem that will have
multiple instances running at the same time, i.e., multiple kernfs_roots.

While investigating the kernfs implementation, we found some global locks
that would cause noticeable lock contention when there are many filesystem
instances.

Fortunately, we found that some optimizations have been made in [1], which
moved kernfs_rwsem into kernfs_root. But there are still some global locks
left.

We think it is also necessary to switch the remaining global locks to
per-fs. Moreover, we strongly agree with Tejun Heo's point in [1]:

  "... this is the right thing to do even if there is no concrete
   performance argument (not saying there isn't). It's just weird to
   entangle these completely unrelated users in a single rwsem."

We think kernfs will be widely used to build other filesystems, so we
strongly recommend switching global locks to per-fs.

Thank you,
Jinliang Zheng :)

[1] https://lore.kernel.org/all/YZbbxK1F7jY%2FRBFF@slm.duckdns.org/

> 
> thanks,
> 
> greg k-h