From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965267AbcA1I5i (ORCPT ); Thu, 28 Jan 2016 03:57:38 -0500 Received: from mx2.parallels.com ([199.115.105.18]:43084 "EHLO mx2.parallels.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752368AbcA1I5g (ORCPT ); Thu, 28 Jan 2016 03:57:36 -0500 Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way To: Minfei Huang References: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> CC: , , , , , , <0x7f454c46@gmail.com>, , , From: Dmitry Safonov Message-ID: <56A9D7F2.5000504@virtuozzo.com> Date: Thu, 28 Jan 2016 11:57:22 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0 MIME-Version: 1.0 In-Reply-To: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-ClientProxiedBy: US-EXCH2.sw.swsoft.com (10.255.249.46) To US-EXCH.sw.swsoft.com (10.255.249.47) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way References: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> From: Dmitry Safonov Message-ID: <56A9D7F2.5000504@virtuozzo.com> Date: Thu, 28 Jan 2016 11:57:22 +0300 MIME-Version: 1.0 In-Reply-To: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-Archive: List-Post: To: Minfei Huang 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 List-ID: 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mx2.parallels.com ([199.115.105.18]) by bombadil.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1aOiOw-0006sv-5j for kexec@lists.infradead.org; Thu, 28 Jan 2016 08:58:02 +0000 Subject: Re: [PATCH] kexec: unmap reserved pages for each error-return way References: <1453895311-11087-1-git-send-email-dsafonov@virtuozzo.com> <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> From: Dmitry Safonov Message-ID: <56A9D7F2.5000504@virtuozzo.com> Date: Thu, 28 Jan 2016 11:57:22 +0300 MIME-Version: 1.0 In-Reply-To: <20160128062906.GA4170@dhcp-128-25.nay.redhat.com> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "kexec" Errors-To: kexec-bounces+dwmw2=infradead.org@lists.infradead.org To: Minfei Huang 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/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 _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec