| Message ID | 58F8D8560200007800152761@prv-mh.provo.novell.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 20/04/17 14:48, Jan Beulich wrote: > Match rcu_lock_domain(), and remove the slightly misleading comment: > This isn't just the companion to rcu_lock_domain_by_id() (and that > latter function indeed also keeps the domain locked, not the domain > list). > > No functional change, as rcu_read_{,un}lock() ignore their arguments > anyway. > > Reported-by: Jann Horn <jannh@google.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although I think it is worth stating in the commit message that the only reason this currently works is because rcu_read_unlock() is entirely empty. > > --- a/xen/include/xen/sched.h > +++ b/xen/include/xen/sched.h > @@ -584,11 +584,10 @@ int rcu_lock_remote_domain_by_id(domid_t > */ > int rcu_lock_live_remote_domain_by_id(domid_t dom, struct domain **d); > > -/* Finish a RCU critical region started by rcu_lock_domain_by_id(). */ > static inline void rcu_unlock_domain(struct domain *d) > { > if ( d != current->domain ) > - rcu_read_unlock(&domlist_read_lock); > + rcu_read_unlock(d); > } > > static inline struct domain *rcu_lock_domain(struct domain *d) > > >
>>> On 20.04.17 at 16:11, <andrew.cooper3@citrix.com> wrote: > On 20/04/17 14:48, Jan Beulich wrote: >> Match rcu_lock_domain(), and remove the slightly misleading comment: >> This isn't just the companion to rcu_lock_domain_by_id() (and that >> latter function indeed also keeps the domain locked, not the domain >> list). >> >> No functional change, as rcu_read_{,un}lock() ignore their arguments >> anyway. I think the second half of this sentence is ... >> Reported-by: Jann Horn <jannh@google.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although I think > it is worth stating in the commit message that the only reason this > currently works is because rcu_read_unlock() is entirely empty. .. what you're asking for here: Whether the function is empty is irrelevant; what is relevant is whether is uses its argument. Jan
Hi, On 20/04/17 15:11, Andrew Cooper wrote: > On 20/04/17 14:48, Jan Beulich wrote: >> Match rcu_lock_domain(), and remove the slightly misleading comment: >> This isn't just the companion to rcu_lock_domain_by_id() (and that >> latter function indeed also keeps the domain locked, not the domain >> list). >> >> No functional change, as rcu_read_{,un}lock() ignore their arguments >> anyway. >> >> Reported-by: Jann Horn <jannh@google.com> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>, although I think > it is worth stating in the commit message that the only reason this > currently works is because rcu_read_unlock() is entirely empty. Release-acked-by: Julien Grall <julien.grall@arm.com> Cheers, > >> >> --- a/xen/include/xen/sched.h >> +++ b/xen/include/xen/sched.h >> @@ -584,11 +584,10 @@ int rcu_lock_remote_domain_by_id(domid_t >> */ >> int rcu_lock_live_remote_domain_by_id(domid_t dom, struct domain **d); >> >> -/* Finish a RCU critical region started by rcu_lock_domain_by_id(). */ >> static inline void rcu_unlock_domain(struct domain *d) >> { >> if ( d != current->domain ) >> - rcu_read_unlock(&domlist_read_lock); >> + rcu_read_unlock(d); >> } >> >> static inline struct domain *rcu_lock_domain(struct domain *d) >> >> >> >
--- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -584,11 +584,10 @@ int rcu_lock_remote_domain_by_id(domid_t */ int rcu_lock_live_remote_domain_by_id(domid_t dom, struct domain **d); -/* Finish a RCU critical region started by rcu_lock_domain_by_id(). */ static inline void rcu_unlock_domain(struct domain *d) { if ( d != current->domain ) - rcu_read_unlock(&domlist_read_lock); + rcu_read_unlock(d); } static inline struct domain *rcu_lock_domain(struct domain *d)
Match rcu_lock_domain(), and remove the slightly misleading comment: This isn't just the companion to rcu_lock_domain_by_id() (and that latter function indeed also keeps the domain locked, not the domain list). No functional change, as rcu_read_{,un}lock() ignore their arguments anyway. Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> correct rcu_unlock_domain() Match rcu_lock_domain(), and remove the slightly misleading comment: This isn't just the companion to rcu_lock_domain_by_id() (and that latter function indeed also keeps the domain locked, not the domain list). No functional change, as rcu_read_{,un}lock() ignore their arguments anyway. Reported-by: Jann Horn <jannh@google.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -584,11 +584,10 @@ int rcu_lock_remote_domain_by_id(domid_t */ int rcu_lock_live_remote_domain_by_id(domid_t dom, struct domain **d); -/* Finish a RCU critical region started by rcu_lock_domain_by_id(). */ static inline void rcu_unlock_domain(struct domain *d) { if ( d != current->domain ) - rcu_read_unlock(&domlist_read_lock); + rcu_read_unlock(d); } static inline struct domain *rcu_lock_domain(struct domain *d)