On Thu, Jul 02, 2015 at 09:45:52AM +0800, Minfei Huang wrote: > For some arch, kexec shall map the reserved pages, then use them, when > we try to start the kdump service. > > Now kexec will never unmap the reserved pages, once it fails to continue > starting the kdump service. > > Make a pair of reserved pages in kdump starting path, whatever kexec > fails or not. > > Signed-off-by: Minfei Huang <mnfhuang at gmail.com> > --- > v2: > - replace the "failure" label with "fail_unmap_pages" > v1: > - reconstruct the patch code > --- > kernel/kexec.c | 26 ++++++++++++++------------ > 1 file changed, 14 insertions(+), 12 deletions(-) > Hi Minfei, I am thinking of moving kernel loading code in a separate function to make things little simpler. Right now it is confusing. Can you please test attached patch. I have only compile tested it. This is primarily doing what you are doing but in a separate function. It seems more readable now. Thanks Vivek --- kernel/kexec.c | 90 +++++++++++++++++++++++++++++++++++---------------------- 1 file changed, 56 insertions(+), 34 deletions(-) Index: rhvgoyal-linux/kernel/kexec.c =================================================================== --- rhvgoyal-linux.orig/kernel/kexec.c 2015-07-06 13:59:35.088129148 -0400 +++ rhvgoyal-linux/kernel/kexec.c 2015-07-07 17:14:23.593175644 -0400 @@ -1247,6 +1247,57 @@ int kexec_load_disabled; static DEFINE_MUTEX(kexec_mutex); +static int __kexec_load(struct kimage **rimage, unsigned long entry, + unsigned long nr_segments, + struct kexec_segment __user * segments, + unsigned long flags) +{ + unsigned long i; + int result; + struct kimage *image; + + if (flags & KEXEC_ON_CRASH) { + /* + * Loading another kernel to switch to if this one + * crashes. Free any current crash dump kernel before + * we corrupt it. + */ + + kimage_free(xchg(&kexec_crash_image, NULL)); + } + + result = kimage_alloc_init(&image, entry, nr_segments, segments, flags); + if (result) + return result; + + if (flags & KEXEC_ON_CRASH) + crash_map_reserved_pages(); + + if (flags & KEXEC_PRESERVE_CONTEXT) + image->preserve_context = 1; + + result = machine_kexec_prepare(image); + if (result) + goto out; + + for (i = 0; i < nr_segments; i++) { + result = kimage_load_segment(image, &image->segment[i]); + if (result) + goto out; + } + + kimage_terminate(image); + *rimage = image; +out: + if (flags & KEXEC_ON_CRASH) + crash_unmap_reserved_pages(); + + /* Free image if there was an error */ + if (result) + kimage_free(image); + return result; +} + SYSCALL_DEFINE4(kexec_load, unsigned long, entry, unsigned long, nr_segments, struct kexec_segment __user *, segments, unsigned long, flags) { @@ -1292,44 +1343,15 @@ SYSCALL_DEFINE4(kexec_load, unsigned lon dest_image = &kexec_image; if (flags & KEXEC_ON_CRASH) dest_image = &kexec_crash_image; - if (nr_segments > 0) { - unsigned long i; - if (flags & KEXEC_ON_CRASH) { - /* - * Loading another kernel to switch to if this one - * crashes. Free any current crash dump kernel before - * we corrupt it. - */ - - kimage_free(xchg(&kexec_crash_image, NULL)); - result = kimage_alloc_init(&image, entry, nr_segments, - segments, flags); - crash_map_reserved_pages(); - } else { - /* Loading another kernel to reboot into. */ - - result = kimage_alloc_init(&image, entry, nr_segments, - segments, flags); - } - if (result) - goto out; - - if (flags & KEXEC_PRESERVE_CONTEXT) - image->preserve_context = 1; - result = machine_kexec_prepare(image); + /* Load new kernel */ + if (nr_segments > 0) { + result = __kexec_load(&image, entry, nr_segments, segments, + flags); if (result) goto out; - - for (i = 0; i < nr_segments; i++) { - result = kimage_load_segment(image, &image->segment[i]); - if (result) - goto out; - } - kimage_terminate(image); - if (flags & KEXEC_ON_CRASH) - crash_unmap_reserved_pages(); } + /* Install the new kernel, and Uninstall the old */ image = xchg(dest_image, image);