Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1320330 > unrolled thread

Re: [PATCH] kexec: unmap reserved pages for each error-return way

Started byMinfei Huang <mhuang@redhat.com>
First post2016-01-28 07:30 +0100
Last post2016-02-02 15:00 +0100
Articles 4 — 3 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] kexec: unmap reserved pages for each error-return way Minfei Huang <mhuang@redhat.com> - 2016-01-28 07:30 +0100
    Re: [PATCH] kexec: unmap reserved pages for each error-return way Dmitry Safonov <dsafonov@virtuozzo.com> - 2016-01-28 10:00 +0100
      Re: [PATCH] kexec: unmap reserved pages for each error-return way Andrew Morton <akpm@linux-foundation.org> - 2016-02-02 06:50 +0100
        Re: [PATCH] kexec: unmap reserved pages for each error-return way Minfei Huang <mhuang@redhat.com> - 2016-02-02 15:00 +0100

#1320330 — Re: [PATCH] kexec: unmap reserved pages for each error-return way

FromMinfei Huang <mhuang@redhat.com>
Date2016-01-28 07:30 +0100
SubjectRe: [PATCH] kexec: unmap reserved pages for each error-return way
Message-ID<qVOfE-7O2-11@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

#1320441

FromDmitry Safonov <dsafonov@virtuozzo.com>
Date2016-01-28 10:00 +0100
Message-ID<qVQAO-Qb-7@gated-at.bofh.it>
In reply to#1320330
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

[toc] | [prev] | [next] | [standalone]

#1323805

FromAndrew Morton <akpm@linux-foundation.org>
Date2016-02-02 06:50 +0100
Message-ID<qXC0F-6aS-5@gated-at.bofh.it>
In reply to#1320441
On Thu, 28 Jan 2016 11:57:22 +0300 Dmitry Safonov <dsafonov@virtuozzo.com> wrote:

> 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.

There were a bunch of patches tossed around in that thread but I'm not
sure that anything actually got applied?  Perhaps some resending is
needed.

[toc] | [prev] | [next] | [standalone]

#1324104

FromMinfei Huang <mhuang@redhat.com>
Date2016-02-02 15:00 +0100
Message-ID<qXJES-3JG-7@gated-at.bofh.it>
In reply to#1323805
On 02/01/16 at 09:45pm, Andrew Morton wrote:
> On Thu, 28 Jan 2016 11:57:22 +0300 Dmitry Safonov <dsafonov@virtuozzo.com> wrote:
> 
> > 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.
> 
> There were a bunch of patches tossed around in that thread but I'm not
> sure that anything actually got applied?  Perhaps some resending is
> needed.
> 

Hi, Andrew.

I will work on it to update a new patch.

Thanks
Minfei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web