[PATCH] x86/HAP: avoid using bogus/misleading locking

[PATCH] x86/HAP: avoid using bogus/misleading locking

[Jan Beulich]

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

hap_teardown() unconditionally releases the paging lock and is always
being called without the lock held: Lock acquire should then be
unconditional too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While this is only a cosmetic change afaict I would still like to
explore whether to include this in 4.9.

--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -576,8 +576,7 @@ void hap_teardown(struct domain *d, bool
     ASSERT(d->is_dying);
     ASSERT(d != current->domain);
 
-    if ( !paging_locked_by_me(d) )
-        paging_lock(d); /* Keep various asserts happy */
+    paging_lock(d); /* Keep various asserts happy */
 
     if ( paging_mode_enabled(d) )
     {




[-- Attachment #2: x86-HAP-teardown-locking.patch --]
[-- Type: text/plain, Size: 762 bytes --]

x86/HAP: avoid using bogus/misleading locking

hap_teardown() unconditionally releases the paging lock and is always
being called without the lock held: Lock acquire should then be
unconditional too.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
While this is only a cosmetic change afaict I would still like to
explore whether to include this in 4.9.

--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -576,8 +576,7 @@ void hap_teardown(struct domain *d, bool
     ASSERT(d->is_dying);
     ASSERT(d != current->domain);
 
-    if ( !paging_locked_by_me(d) )
-        paging_lock(d); /* Keep various asserts happy */
+    paging_lock(d); /* Keep various asserts happy */
 
     if ( paging_mode_enabled(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] x86/HAP: avoid using bogus/misleading locking

[Andrew Cooper]

On 29/05/2017 13:37, Jan Beulich wrote:
> hap_teardown() unconditionally releases the paging lock and is always
> being called without the lock held: Lock acquire should then be
> unconditional too.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH] x86/HAP: avoid using bogus/misleading locking

[Julien Grall]

Hi,

On 05/29/2017 02:55 PM, Andrew Cooper wrote:
> On 29/05/2017 13:37, Jan Beulich wrote:
>> hap_teardown() unconditionally releases the paging lock and is always
>> being called without the lock held: Lock acquire should then be
>> unconditional too.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Assuming this goes through staging first:

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

Re: [PATCH] x86/HAP: avoid using bogus/misleading locking

[Jan Beulich]

>>> On 31.05.17 at 11:03, <julien.grall@arm.com> wrote:
> On 05/29/2017 02:55 PM, Andrew Cooper wrote:
>> On 29/05/2017 13:37, Jan Beulich wrote:
>>> hap_teardown() unconditionally releases the paging lock and is always
>>> being called without the lock held: Lock acquire should then be
>>> unconditional too.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>>
>> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Assuming this goes through staging first:

Of course, but strictly speaking this still needs an ack from George
before it can go in anywhere.

Jan

> Release-acked-by: Julien Grall <julien.grall@arm.com>
> 
> Cheers,
> 
> -- 
> Julien Grall




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

Ping: [PATCH] x86/HAP: avoid using bogus/misleading locking

[Jan Beulich]

>>> On 29.05.17 at 14:37, <JBeulich@suse.com> wrote:
> hap_teardown() unconditionally releases the paging lock and is always
> being called without the lock held: Lock acquire should then be
> unconditional too.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> While this is only a cosmetic change afaict I would still like to
> explore whether to include this in 4.9.
> 
> --- a/xen/arch/x86/mm/hap/hap.c
> +++ b/xen/arch/x86/mm/hap/hap.c
> @@ -576,8 +576,7 @@ void hap_teardown(struct domain *d, bool
>      ASSERT(d->is_dying);
>      ASSERT(d != current->domain);
>  
> -    if ( !paging_locked_by_me(d) )
> -        paging_lock(d); /* Keep various asserts happy */
> +    paging_lock(d); /* Keep various asserts happy */
>  
>      if ( paging_mode_enabled(d) )
>      {




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