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

[Dmitry Safonov]

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