[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level



On 04/19/2017 10:47 AM, Eric DeVolder wrote:
The spinlock in kexec_swap_images() was removed as
this function is only reachable on the kexec hypercall, which is
now protected at the top-level in do_kexec_op_internal(),
thus the local spinlock is no longer necessary.

Per recommendation from Jan Beulich and Andrew Cooper, I left
an ASSERT in place of the spin_lock().

Signed-off-by: Eric DeVolder <eric.devolder@xxxxxxxxxx>
Reviewed-by: Bhavesh Davda <bhavesh.davda@xxxxxxxxxx>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
---
v3:
 - Incorporated feedback from Jan Beulich and Andrew Cooper
   to leave an ASSERT where spin_lock() once was.

v2: 04/17/2017
 - Patch titled 'kexec: use hypercall_create_continuation to protect KEXEC ops'
 - Separated removal of spinlock in kexec_swap_images() into its own patch.

v1: 04/10/2017
 - Patch titled 'kexec: Add spinlock for the whole hypercall'
 - Removal of spinlock in kexec_swap_images() was part of other patch.
---
 xen/common/kexec.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 253c204..af32e58 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -820,7 +820,6 @@ static int kexec_exec(XEN_GUEST_HANDLE_PARAM(void) uarg)
 static int kexec_swap_images(int type, struct kexec_image *new,
                              struct kexec_image **old)
 {
-    static DEFINE_SPINLOCK(kexec_lock);
     int base, bit, pos;
     int new_slot, old_slot;

@@ -832,7 +831,7 @@ static int kexec_swap_images(int type, struct kexec_image 
*new,
     if ( kexec_load_get_bits(type, &base, &bit) )
         return -EINVAL;

-    spin_lock(&kexec_lock);
+    ASSERT( !test_bit(KEXEC_FLAG_IN_HYPERCALL, &kexec_flags) );

There are two problems here: the spaces need to be eliminated, and more importantly, the sense/polarity of the expression is wrong, the '!' needs to be removed.


     pos = (test_bit(bit, &kexec_flags) != 0);
     old_slot = base + pos;
@@ -846,8 +845,6 @@ static int kexec_swap_images(int type, struct kexec_image 
*new,
     clear_bit(old_slot, &kexec_flags);
     *old = kexec_image[old_slot];

-    spin_unlock(&kexec_lock);
-
     return 0;
 }




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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.