[PATCH] correct rcu_unlock_domain()

[PATCH] correct rcu_unlock_domain()

[Jan Beulich]

[-- Attachment #1: Type: text/plain, Size: 922 bytes --]

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)




[-- Attachment #2: rcu-unlock-domain.patch --]
[-- Type: text/plain, Size: 947 bytes --]

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)

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] correct rcu_unlock_domain()

[Andrew Cooper]

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)
>
>
>


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] correct rcu_unlock_domain()

[Jan Beulich]

>>> 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


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH] correct rcu_unlock_domain()

[Julien Grall]

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)
>>
>>
>>
>

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel