[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 2/2] kexec: remove spinlock now that all KEXEC hypercall ops are protected at the top-level
On Wed, Apr 19, 2017 at 05:20:50AM -0600, Jan Beulich wrote: > >>> On 19.04.17 at 12:56, <daniel.kiper@xxxxxxxxxx> wrote: > > On Tue, Apr 18, 2017 at 04:49:48AM -0600, Jan Beulich wrote: > >> >>> On 17.04.17 at 21:09, <eric.devolder@xxxxxxxxxx> 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. > >> > >> But perhaps leave an ASSERT() there, making sure the in-hypercall > >> flag is set? > > > > I am not sure why but if at all I think that we should also consider > > other key kexec functions then. Or put ASSERT() into do_kexec_op_internal() > > just before "switch ( op )". > > The point of my placement suggestion was that the ASSERT() > effectively replaces the lock acquire - the places you name > didn't previously require any synchronization. After the first patch of this series kexec_swap_images() cannot be started twice in parallel. So, I do not see the point of ASSERT() here. Or let's say we wish to have it to double check that "the in-hypercall flag is set". AIUI, it is your original idea. However, then I think that we should have an ASSERT() at least in kexec_load_slot() because parallel loads make issues too. And we can go higher to feel more safe. That is why I suggested do_kexec_op_internal() as the final resting place for an ASSERT(). Simply it looks to me the safest approach. Am I missing something? Daniel _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |