Re: [PATCH] kernfs: Use RCU for kernfs_node::name lookup.
From: Sebastian Andrzej Siewior
Date: Mon Nov 11 2024 - 12:05:04 EST
On 2024-11-08 12:31:22 [-1000], Tejun Heo wrote:
> Hello, Sebastian.
Hi Tejun,
> On Fri, Nov 08, 2024 at 11:24:06PM +0100, Sebastian Andrzej Siewior wrote:
> > Instead of using kernfs_rename_lock for lookups of ::name use RCU for
> > lookup. Rely on kn's kernfs_root::kernfs_rwsem for update
> > synchronisation.
> >
> > The .*_locked() have been moved into the callers.
> > The lock in kernfs_get_parent() has been dropped, the parent node should
> > node vanish underneath us. The RCU read-lock and atomic_inc_not_zero()
> > is a safety net in case it does.
> > kernfs_fop_readdir() no longer drops kernfs_root::kernfs_rwsem to ensure
> > the name pointer does not vanish while the page fault is handled.
> > kernfs_notify_workfn() gained the lock for the same reason.
>
> I owe an apology. I was just thinking about cgroups. Sysfs, I think, does
no worries.
> allow moving node a different parent, which IIRC is used by netdevs. How
> about something like this:
>
> - Add a KERNFS_ROOT flag indicating that parent-changing moves aren't
> allowed.
>
> - Update kernfs_rename_ns() to trigger warning and fail if the above flag is
> set and new_parent is different from the old one.
>
> - Create a separate interface which uses RCU instead of rename lock for name
> / path lookups. The RCU ones should trigger warning if used when the above
> KERNFS_ROOT flag is not set.
Let me check if I understood. We have three users of kernfs:
- cgroup
cgroup1_rename(): parent check (would get the new KERNFS_ROOT flag)
- resctrl
rdtgroup_rename(): seems to allow any "mon group" directory
different parent possible.
- sysfs
sysfs_move_dir_ns(): reame to a different parent
That new RCU interface would be used by cgroup only and sysfs/ resctrl
would remain using the "old" code?
If so, would you prefer
|struct kernfs_node {
| …
| union {
| const char name;
| const char __rcu *name_rcu;
| }
to avoid patching resctrl + sysfs for for the rcu_derference name
lookup?
> Thanks.
Sebastian