Messages in this thread | | | Date | Tue, 14 Jun 2022 06:20:21 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH v6 1/4] kernfs: make ->attr.open RCU protected. |
| |
On Tue, Jun 14, 2022 at 05:03:43PM +1000, Imran Khan wrote: > +/** > + * kernfs_deref_open_node_protected - Get kernfs_open_node corresponding to @kn > + * > + * @kn: target kernfs_node. > + * > + * Fetch and return ->attr.open of @kn when caller holds the > + * kernfs_open_file_mutex. > + * > + * Update of ->attr.open happens under kernfs_open_file_mutex. So when > + * the caller guarantees that this mutex is being held, other updaters can't > + * change ->attr.open and this means that we can safely deref ->attr.open > + * outside RCU read-side critical section. > + * > + * The caller needs to make sure that kernfs_open_file_mutex is held. > + */ > +static struct kernfs_open_node * > +kernfs_deref_open_node_protected(struct kernfs_node *kn) > +{ > + return rcu_dereference_check(kn->attr.open, > + lockdep_is_held(&kernfs_open_file_mutex));
Hey, so, the difference between rcu_dereference_check() and rcu_dereference_protected() is that the former can be called either with rcu read locked or under the extra condition (here, open_file_mutex held) while the latter can't be used under rcu read lock. The two can generate different codes too - the former enforces dependency ordering which makes accesses under rcu read lock safe, while the latter doesn't.
In the above, you're saying that the accessor is only to be used while holding kernfs_open_file_mutex but then using rcu_dereference_check() which is odd. There are two ways you can go 1. ensure that the accessor is always used under the mutex and use rcu_dereference_protected() or 2. if the function can be used under rcu read lock, rename so that the differentiation between the two accessors is based on the parameter type, not whether they're protected or not.
Can you please post the updated patch as a reply to this one? No need to post the whole thing over and over again.
Thanks.
-- tejun
| |