From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754954AbcA1GZ7 (ORCPT ); Thu, 28 Jan 2016 01:25:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:53208 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754561AbcA1GZ6 (ORCPT ); Thu, 28 Jan 2016 01:25:58 -0500 Date: Thu, 28 Jan 2016 14:29:06 +0800 From: Minfei Huang To: Dmitry Safonov 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 Message-ID: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> References: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx1.redhat.com ([209.132.183.28]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aOg27-0003mR-4A for kexec@lists.infradead.org; Thu, 28 Jan 2016 06:26:19 +0000 Date: Thu, 28 Jan 2016 14:29:06 +0800 From: Minfei Huang Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way Message-ID: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> References: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Dmitry Safonov 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 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