On Tue, Mar 21, 2017 at 06:25:46PM +0000, James Morse wrote: > Hi Akashi, > > On 15/03/17 09:59, AKASHI Takahiro wrote: > > Since arch_kexec_protect_crashkres() removes a mapping for crash dump > > kernel image, the loaded data won't be preserved around hibernation. > > > > In this patch, helper functions, kexec_prepare_suspend()/ > > kexec_post_resume(), are additionally called before/after hibernation so > > that the relevant memory segments will be mapped again and preserved just > > as the others are. > > > > In addition, to minimize the size of hibernation image, > > kexec_is_chraskres_nosave() is added to pfn_is_nosave() in order to > > (crashkres)
Yes. > > recoginize only the pages that hold loaded crash dump kernel image as > > (recognize) Ah, yes ... > > saveable. Hibernation excludes any pages that are marked as Reserved and > > yet "nosave." > > > Neat! I didn't think this would be possible without hacking > kernel/power/snapshot.c. > > I've given this a spin on Juno and Seattle, I even added debug_pagealloc, but > that doesn't trick it because your kexec_prepare_suspend() puts the mapping > back. > > Reviewed-by: James Morse <[email protected]> > > > Thanks, > > James > > > Some nits about comments: > > > diff --git a/arch/arm64/kernel/hibernate.c b/arch/arm64/kernel/hibernate.c > > index 97a7384100f3..1e10fafa59bd 100644 > > --- a/arch/arm64/kernel/hibernate.c > > +++ b/arch/arm64/kernel/hibernate.c > > @@ -286,6 +288,9 @@ int swsusp_arch_suspend(void) > > local_dbg_save(flags); > > > > if (__cpu_suspend_enter(&state)) { > > + /* make the crash dump kernel image visible/saveable */ > > + kexec_prepare_suspend(); > > Strictly this is kdump not kexec, but the comment makes that clear. I hesitated to use "kdump_" here as there are no functions with such a prefix (say, even arch_kexec_protect_crashkres()). So probably "crash_" or "crash_kexec_" would sound much better. > > + > > sleep_cpu = smp_processor_id(); > > ret = swsusp_save(); > > } else { > > > diff --git a/arch/arm64/kernel/machine_kexec.c > > b/arch/arm64/kernel/machine_kexec.c > > index 02e4f929db3b..82f48db589cf 100644 > > --- a/arch/arm64/kernel/machine_kexec.c > > +++ b/arch/arm64/kernel/machine_kexec.c > > > @@ -220,7 +221,6 @@ void arch_kexec_protect_crashkres(void) > > kexec_crash_image->segment[i].memsz, > > PAGE_KERNEL_INVALID, true); > > > > - > > Stray whitespace change from a previous patch? Oops, fix it. > > flush_tlb_all(); > > } > > > > @@ -233,3 +233,74 @@ void arch_kexec_unprotect_crashkres(void) > > > +/* > > + * kexec_is_crashres_nosave > > + * > > + * Return true only if a page is part of reserved memory for crash dump > > kernel, > > + * but does not hold any data of loaded kernel image. > > + * > > + * Note that all the pages in crash dump kernel memory have been initially > > + * marked as Reserved in kexec_reserve_crashkres_pages(). > > + * > > + * In hibernation, the pages which are Reserved and yet "nosave" > > + * are excluded from the hibernation iamge. kexec_is_crashkres_nosave() > > + * does this check for crash dump kernel and will reduce the total size > > + * of hibernation image. > > + */ > > + > > +bool kexec_is_crashkres_nosave(unsigned long pfn) > > +{ > > + int i; > > + phys_addr_t addr; > > + > > + /* in reserved memory? */ > > Comment in the wrong place? This is after my deep thinking :) but > > > + if (!crashk_res.end) > > + return false; > > + > > + addr = __pfn_to_phys(pfn); > > (makes more sense here) if you think so, I will follow you. > > > + if ((addr < crashk_res.start) || (crashk_res.end < addr)) > > + return false; > > + > > > + /* not part of loaded kernel image? */ > > Comment in the wrong place? > > > > + if (!kexec_crash_image) > > + return true; > > + > > (makes more sense here) ditto > > > + for (i = 0; i < kexec_crash_image->nr_segments; i++) > > + if (addr >= kexec_crash_image->segment[i].mem && > > + addr < (kexec_crash_image->segment[i].mem + > > + kexec_crash_image->segment[i].memsz)) > > + return false; > > + > > + return true; > > +} > > + > > > Thanks, > > James Thank you for your review! -Takahiro AKASHI _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
