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
