Re: [PATCH] kexec: unmap reserved pages for each error-return way

From: Dmitry Safonov
Date: Thu Jan 28 2016 - 03:57:41 EST


On 01/28/2016 09:29 AM, Minfei Huang wrote:
On 01/27/16 at 02:48pm, Dmitry Safonov wrote:
For allocation of kimage failure or kexec_prepare or load segments
errors there is no need to keep crashkernel memory mapped.
It will affect only s390 as map/unmap hook defined only for it.
As on unmap s390 also changes os_info structure let's check return code
and add info only on success.
Hi, Dmitry.

Previously, I sent a patch to fix this issue. You can refer it in
following link.

http://lists.infradead.org/pipermail/kexec/2015-July/013960.html
Oh, scratch my patch - I'm fine with yours, wanted to do the similar thing
because it has dazzled me while I was debugging around.

And this patch is fixed from kexec.

If crash_map_reserved_pages fails to map reserved memory, is it
necessary to continue the process on s390? If no, it is better to enter
the error handle path, then return. Thus there is no need to pass the
parameter to indicate the error or not.

@@ -147,39 +147,34 @@ static int kdump_csum_valid(struct kimage *image)
}
/*
- * Map or unmap crashkernel memory
+ * Map crashkernel memory
*/
-static void crash_map_pages(int enable)
+void crash_map_reserved_pages(void)
{
unsigned long size = resource_size(&crashk_res);
BUG_ON(crashk_res.start % KEXEC_CRASH_MEM_ALIGN ||
size % KEXEC_CRASH_MEM_ALIGN);
- if (enable)
- vmem_add_mapping(crashk_res.start, size);
- else {
- vmem_remove_mapping(crashk_res.start, size);
- if (size)
- os_info_crashkernel_add(crashk_res.start, size);
- else
- os_info_crashkernel_add(0, 0);
- }
-}
-
-/*
- * Map crashkernel memory
- */
-void crash_map_reserved_pages(void)
-{
- crash_map_pages(1);
+ vmem_add_mapping(crashk_res.start, size);
}
It is fine to cleanup this function. And add the logic into function
crash_unmap_reserved_pages.

/*
* Unmap crashkernel memory
*/
-void crash_unmap_reserved_pages(void)
+void crash_unmap_reserved_pages(int error)
{
- crash_map_pages(0);
+ unsigned long size = resource_size(&crashk_res);
+
+ BUG_ON(crashk_res.start % KEXEC_CRASH_MEM_ALIGN ||
+ size % KEXEC_CRASH_MEM_ALIGN);
+ vmem_remove_mapping(crashk_res.start, size);
+
+ if (error)
+ return;
+ if (size)
+ os_info_crashkernel_add(crashk_res.start, size);
+ else
+ os_info_crashkernel_add(0, 0);
}
/*
Thanks
Minfei


--
Regards,
Dmitry Safonov