Messages in this thread | | | Date | Tue, 26 Apr 2022 08:37:30 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH v8 02/10] kernfs: make ->attr.open RCU protected. |
| |
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
| |