Re: [PATCH v3] kexec: Make a pair of map and unmap reserved pages when kdump fails to start

From: Vivek Goyal
Date: Tue Jul 07 2015 - 17:20:16 EST


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@xxxxxxxxx>
> ---
> 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);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/