[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <7B510F85-E964-43FF-A418-7A6CEF8B0308@meta.com>
Date: Thu, 19 Jun 2025 15:33:14 +0000
From: Song Liu <songliubraving@...a.com>
To: Christian Brauner <brauner@...nel.org>
CC: Song Liu <song@...nel.org>,
Greg Kroah-Hartman
<gregkh@...uxfoundation.org>,
Tejun Heo <tj@...nel.org>, "bpf@...r.kernel.org" <bpf@...r.kernel.org>,
"linux-fsdevel@...r.kernel.org"
<linux-fsdevel@...r.kernel.org>,
"linux-kernel@...r.kernel.org"
<linux-kernel@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
Kernel Team <kernel-team@...a.com>,
"andrii@...nel.org" <andrii@...nel.org>,
"eddyz87@...il.com"
<eddyz87@...il.com>,
"ast@...nel.org" <ast@...nel.org>,
"daniel@...earbox.net" <daniel@...earbox.net>,
"martin.lau@...ux.dev"
<martin.lau@...ux.dev>,
"viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
"jack@...e.cz" <jack@...e.cz>,
"kpsingh@...nel.org" <kpsingh@...nel.org>,
"mattbobrowski@...gle.com" <mattbobrowski@...gle.com>,
"amir73il@...il.com"
<amir73il@...il.com>,
"daan.j.demeyer@...il.com" <daan.j.demeyer@...il.com>
Subject: Re: [PATCH bpf-next 1/4] kernfs: Add __kernfs_xattr_get for RCU
protected access
> On Jun 19, 2025, at 3:01 AM, Christian Brauner <brauner@...nel.org> 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@...nel.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.
>
> You should then just use READ_ONCE(kn->iattr) or the
> kernfs_iattrs_noalloc(kn) helper in your kfunc.
> <0001-kernfs-remove-iattr_mutex.patch>
This looks better indeed.
I will drop 1/4 and include this patch.
Thanks,
Song
Powered by blists - more mailing lists