[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20160323024836.GC2567@x1.redhat.com>
Date: Wed, 23 Mar 2016 10:48:36 +0800
From: Baoquan He <bhe@...hat.com>
To: Xunlei Pang <xlpang@...hat.com>
Cc: Minfei Huang <mnfhuang@...il.com>, ebiederm@...ssion.com,
akpm@...ux-foundation.org, mhuang@...hat.com,
kexec@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2 0/2] kexec: Make a pair of map/unmap reserved pages in
error path
On 03/01/16 at 05:53pm, Xunlei Pang wrote:
> This is a bug fix.
>
> After this, I will try to do a cleanup for crash_unmap/map_reserved_pages()
> (only used by S390) to consolidate it with arch_kexec_unprotect/protect_crashkres().
Hi Xunlei, Minfei,
I think you need discuss together about how to do clean up codes in this
place. From my point of view, arch_map/unmap_reserved_pages and
arch_kexec_protect/unprotect_crashkres() are for the same goal but by
different ways on different arch. So for Xunlei's patchset, you might
need to rethink your implementation, the name of function. I personally
think you just implement a x86 specific arch_map/unmap_reserved_pages.
It may need a more generic name, and then add your x86 arch specific
implementation. Sorry I can't see your patches on my mail client,
Xunlei. Since Andrew asked, I just checked these.
I am fine with Minfei's patch 1/2. But for patch 2/2, it's a little
comfortable to me. Is it really necessary to abstract code block from
kexec_load, then wrap them into a newly added function do_kexec_load()?
Without this wrapping is there a way to do your bug fix? Is there
possibility that do_kexec_load will be called in other places? What's
the benefit to wrap it into do_kexec_load against not wrapping?
Thanks
Baoquan
>
> Regards,
> Xunlei
>
> On 03/01/2016 at 04:02 PM, Minfei Huang wrote:
> > v1:
> > - Bisect the patch according to Andrew Morton's suggestion
> >
> > Minfei Huang (2):
> > kexec: Make a pair of map/unmap reserved pages in error path
> > kexec: Do a cleanup for function kexec_load
> >
> > kernel/kexec.c | 112 ++++++++++++++++++++++++++++++++-------------------------
> > 1 file changed, 63 insertions(+), 49 deletions(-)
> >
>
>
> _______________________________________________
> kexec mailing list
> kexec@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
Powered by blists - more mailing lists