On Tue, 14 Jul 2015 10:09:20 -0400 Vivek Goyal <vgoyal at redhat.com> wrote: > On Fri, Jul 10, 2015 at 11:14:06AM +0200, Michael Holzheu wrote: > > [..] > > What about the following patch: > > --- > > diff --git a/kernel/kexec.c b/kernel/kexec.c > > index 7a36fdc..7837c4e 100644 > > --- a/kernel/kexec.c > > +++ b/kernel/kexec.c > > @@ -1236,10 +1236,68 @@ int kexec_load_disabled; > > > > static DEFINE_MUTEX(kexec_mutex); > > > > +static int __kexec_load(unsigned long entry, unsigned long nr_segments, > [snip] > > + > > +failure_unmap_mem: > > I don't like this tag "failure_unmap_mem". We are calling this both > in success path as well as failure path. So why not simply call it "out". Since the code is better readable now, I'm fine with "out" :-) > > > + if (flags & KEXEC_ON_CRASH) > > + crash_unmap_reserved_pages(); > > + kimage_free(image); > > Now kimage_free() is called with kexec_mutex held. Previously that was > not the case. I hope that's not a problem. Yes, I noticed that. But also in the original code there is already one spot where kimage_free() is called under lock: /* * In case of crash, new kernel gets loaded in reserved region. It is * same memory where old crash kernel might be loaded. Free any * current crash dump kernel before we corrupt it. */ if (flags & KEXEC_FILE_ON_CRASH) kimage_free(xchg(&kexec_crash_image, NULL)); Therefore I thought it should be ok. Michael