Re: [PATCH bpf-next 1/4] kernfs: Add __kernfs_xattr_get for RCU protected access

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Thu, Jun 19, 2025 at 12:01:19PM +0200, Christian Brauner wrote:
> On Wed, Jun 18, 2025 at 04:37:36PM -0700, Song Liu wrote:
> > Existing kernfs_xattr_get() locks iattr_mutex, so it cannot be used in
> > RCU critical sections. Introduce __kernfs_xattr_get(), which reads xattr
> > under RCU read lock. This can be used by BPF programs to access cgroupfs
> > xattrs.
> > 
> > Signed-off-by: Song Liu <song@kernel.org>
> > ---
> >  fs/kernfs/inode.c      | 14 ++++++++++++++
> >  include/linux/kernfs.h |  2 ++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c
> > index b83054da68b3..0ca231d2012c 100644
> > --- a/fs/kernfs/inode.c
> > +++ b/fs/kernfs/inode.c
> > @@ -302,6 +302,20 @@ int kernfs_xattr_get(struct kernfs_node *kn, const char *name,
> >  	return simple_xattr_get(&attrs->xattrs, name, value, size);
> >  }
> >  
> > +int __kernfs_xattr_get(struct kernfs_node *kn, const char *name,
> > +		       void *value, size_t size)
> > +{
> > +	struct kernfs_iattrs *attrs;
> > +
> > +	WARN_ON_ONCE(!rcu_read_lock_held());
> > +
> > +	attrs = rcu_dereference(kn->iattr);
> > +	if (!attrs)
> > +		return -ENODATA;
> 
> Hm, that looks a bit silly. Which isn't your fault. I'm looking at the
> kernfs code that does the xattr allocations and I think that's the
> origin of the silliness. It uses a single global mutex for all kernfs
> users thus serializing all allocations for kernfs->iattr. That seems
> crazy but maybe I'm missing a good reason.
> 
> I'm appending a patch to remove that mutex. @Greg, @Tejun, can you take
> a look whether that makes sense to you. Then I can take that patch and
> you can build yours on top of the series and I'll pick it all up in one
> go.

Looks sane to me, thanks!

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>