Re: [PATCH] kernfs: Use RCU for kernfs_node::name lookup.

[Sebastian Andrzej Siewior]

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