From: Minfei Huang <mhuang@redhat.com> To: Dmitry Safonov <dsafonov@virtuozzo.com> Cc: linux-kernel@vger.kernel.org, schwidefsky@de.ibm.com, heiko.carstens@de.ibm.com, ebiederm@xmission.com, akpm@linux-foundation.org, linux-s390@vger.kernel.org, 0x7f454c46@gmail.com, kexec@lists.infradead.org, holzheu@linux.vnet.ibm.com, dyoung@redhat.com Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way Date: Thu, 28 Jan 2016 14:29:06 +0800 [thread overview] Message-ID: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> (raw) In-Reply-To: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> 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 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
WARNING: multiple messages have this Message-ID (diff)
From: Minfei Huang <mhuang@redhat.com> To: Dmitry Safonov <dsafonov@virtuozzo.com> Cc: linux-s390@vger.kernel.org, 0x7f454c46@gmail.com, heiko.carstens@de.ibm.com, linux-kernel@vger.kernel.org, ebiederm@xmission.com, schwidefsky@de.ibm.com, akpm@linux-foundation.org, holzheu@linux.vnet.ibm.com, dyoung@redhat.com, kexec@lists.infradead.org Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way Date: Thu, 28 Jan 2016 14:29:06 +0800 [thread overview] Message-ID: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> (raw) In-Reply-To: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> 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 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 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
next prev parent reply other threads:[~2016-01-28 6:25 UTC|newest] Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-01-27 11:48 [PATCH] kexec: unmap reserved pages for each error-return way Dmitry Safonov 2016-01-27 11:48 ` Dmitry Safonov 2016-01-27 11:48 ` Dmitry Safonov 2016-01-27 19:15 ` Andrew Morton 2016-01-27 19:15 ` Andrew Morton 2016-01-27 19:15 ` Andrew Morton 2016-01-28 10:32 ` Michael Holzheu 2016-01-28 10:32 ` Michael Holzheu 2016-01-28 10:32 ` Michael Holzheu 2016-01-28 11:56 ` Xunlei Pang 2016-01-28 11:56 ` Xunlei Pang 2016-01-28 12:44 ` Michael Holzheu 2016-01-28 12:44 ` Michael Holzheu 2016-01-28 13:12 ` Xunlei Pang 2016-01-28 13:12 ` Xunlei Pang 2016-01-28 14:01 ` Michael Holzheu 2016-01-28 14:01 ` Michael Holzheu [not found] ` <56A983F3.5010506@redhat.com> [not found] ` <56A9D927.70402@virtuozzo.com> 2016-01-29 3:14 ` Xunlei Pang 2016-01-29 3:14 ` Xunlei Pang 2016-01-28 3:36 ` Xunlei Pang 2016-01-28 3:36 ` Xunlei Pang 2016-01-28 6:29 ` Minfei Huang [this message] 2016-01-28 6:29 ` Minfei Huang 2016-01-28 8:57 ` Dmitry Safonov 2016-01-28 8:57 ` Dmitry Safonov 2016-01-28 8:57 ` Dmitry Safonov 2016-02-02 5:45 ` Andrew Morton 2016-02-02 5:45 ` Andrew Morton 2016-02-02 5:45 ` Andrew Morton 2016-02-02 13:56 ` Minfei Huang 2016-02-02 13:56 ` Minfei Huang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160128062906.GA4170@dhcp-128-25.nay.redhat.com \ --to=mhuang@redhat.com \ --cc=0x7f454c46@gmail.com \ --cc=akpm@linux-foundation.org \ --cc=dsafonov@virtuozzo.com \ --cc=dyoung@redhat.com \ --cc=ebiederm@xmission.com \ --cc=heiko.carstens@de.ibm.com \ --cc=holzheu@linux.vnet.ibm.com \ --cc=kexec@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-s390@vger.kernel.org \ --cc=schwidefsky@de.ibm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.