Re: [PATCH 2/2] fs/proc: take rcu_read_lock() in proc_sys_compare()

[NeilBrown]

On Thu, 12 Jun 2025, Al Viro wrote:
> On Thu, Jun 12, 2025 at 08:57:03AM +1000, NeilBrown wrote:
> 
> > However there is no guarantee that this lock is held by d_same_name()
> > (the caller of ->d_compare).  In particularly d_alloc_parallel() calls
> > d_same_name() after rcu_read_unlock().
> 
> d_alloc_parallel() calls d_same_name() with dentry being pinned;
> if it's positive, nothing's going to happen to its inode,
> rcu_read_lock() or not.  It can go from negative to positive,
> but that's it.
> 
> Why is it needed?  We do care about possibly NULL inode (basically,
> when RCU dcache lookup runs into a dentry getting evicted right
> under it), but that's not relevant here.
> 

Maybe it isn't needed.  Maybe I could fix the warning by removing the
rcu_dereference() (and the RCU_INIT_POINTER() in inode.c).  But then I
might have to pretend that I understand the code - and it makes no
sense.

If a second d_alloc_parallel() is called while there is already a
d_in_lookup() dentry, then ->d_compare will return 1 so a second
d_in_lookup() will be created and ->lookup will be called twice
(possibly concurrently) and both will be added to the dcache.  Probably
not harmful but not really wanted.

And I'm having trouble seeing how sysctl_is_seen() is useful.  If it
reports that the sysctl is not visible to this process, it'll just
create a new dentry/inode which is that same as any other that would be
created... 

NeilBrown