Re: [PATCH v8 02/10] kernfs: make ->attr.open RCU protected.

[Tejun Heo <tj@kernel.org>]

On Tue, Apr 26, 2022 at 11:54:36AM +1000, Imran Khan wrote:
> >> @@ -912,14 +920,13 @@ void kernfs_notify(struct kernfs_node *kn)
> >>  		return;
> >>  
> >>  	/* kick poll immediately */
> >> -	spin_lock_irqsave(&kernfs_open_node_lock, flags);
> >> -	on = kn->attr.open;
> >> +	rcu_read_lock();
> >> +	on = rcu_dereference(kn->attr.open);
> >>  	if (on) {
> >>  		atomic_inc(&on->event);
> >>  		wake_up_interruptible(&on->poll);
> >>  	}
> >> -	spin_unlock_irqrestore(&kernfs_open_node_lock, flags);
> >> -
> >> +	rcu_read_unlock();
> > 
> > An explanation of why this is safe in terms of event ordering would be great
> > here.
> > 
> 
> This is safe because here we don't need to refcnt ->on in this case. If
> writer (kernfs_put_open_node) has already made ->attr.open NULL we will
> bail out. If kernfs_notify got an old ->attr.open we can still safely
> process the event, even if kernfs_put_open_node updates ->attr.open to
> NULL in parallel.
> In both the cases the behaviour/order will be same as earlier code that
> used kernfs_open_node_lock.
> Please let me know if this answers your query or if something is still
> missing.

I was more wondering whether if there's someone waiting for an event which
should arrive considering the sequence of events, whether the lockless code
would have the same ordering properties. I think it does given that the
event sequence can only be defined in memory visibility terms anyway.

Thanks.

-- 
tejun