Messages in this thread | | | Date | Sun, 12 Jun 2022 07:58:37 -1000 | From | Tejun Heo <> | Subject | Re: [PATCH RESEND v4 1/4] kernfs: make ->attr.open RCU protected. |
| |
Hello,
Sorry about the long delay.
On Thu, Jun 02, 2022 at 04:39:04PM +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(writer) holds > + * kernfs_open_file_mutex. > + * > + * Update of ->attr.open happens under kernfs_open_file_mutex. So as long as > + * the current updater (caller) is holding this mutex, other updaters will not > + * be able to change ->attr.open and this means that we can safely deref > + * ->attr.open outside RCU read-side critical section. > + * > + * This should ONLY be used by updaters of ->attr.open and 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_protected(kn->attr.open, > + lockdep_is_held(&kernfs_open_file_mutex)); > +} > + > +/** > + * kernfs_check_open_node_protected - Get kernfs_open_node corresponding to @kn > + * > + * @kn: target kernfs_node. > + * > + * Fetch and return ->attr.open of @kn when caller(reader) holds > + * kernfs_open_file_mutex. > + * > + * Update of ->attr.open happens under kernfs_open_file_mutex. So as long as > + * the current reader (caller) is holding this mutex, updaters will not be > + * able to change ->attr.open and this means that we can safely deref > + * ->attr.open outside RCU read-side critical section. > + * > + * This should ONLY be used by readers of ->attr.open and caller needs to make > + * sure that kernfs_open_file_mutex is held. > + */ > +static struct kernfs_open_node * > +kernfs_check_open_node_protected(struct kernfs_node *kn) > +{ > + return rcu_dereference_check(kn->attr.open, > + lockdep_is_held(&kernfs_open_file_mutex)); > +}
I don't understand why the above is necessary. Whether you're a reader or writer, you can deref the pointer w/ _protected as long as you're holding the lock, right? If I'm mistaken and somehow a reader needs to use a deref_check, I don't see a reason for this to be a separate accessor. Why not just merge the condition into the kernfs_deref_open_node()?
Thanks.
-- tejun
| |