Re: [PATCH] mips: kernel: setup: fix crash kernel resource allocation

[Ivan Khoronzhuk]

On Sun, Feb 07, 2021 at 11:18:42AM +0200, Mike Rapoport wrote:
> On Sat, Feb 06, 2021 at 12:59:40PM +0000, Ivan Khoronzhuk wrote:
> > In order to avoid crash kernel corruption, its memory is reserved
> > early in memblock and as result, in time when resources are inited
> > it's not present in memblock.memory, so crash kernel memory is out
> > of ranges listed with for_each_mem_range(). To avoid it and still
> > keep memory reserved lets reseve it out of loop by inserting it in
> > iomem_resource.
>
> Unless I misread the code, the crash kernel memory is actually allocated
> from memblock (memblock_find_in_range + memblock_reserve), but for some
> reason memblock_reserve(<crash kernel>) is called outside
> mips_parse_crashkernel(). So the crash kernel memory is surely in both
> memblock.memory and memblock.reserved and it will be covered by
> for_each_mem_range().
>
> The mips_parse_crashkernel() function and the following reservation of
> crash kernel memory should be merged, IMO, and this can be further
> simplified with memblock_alloc() helpers.
>
> Is there a particular issue you are trying to fix?

Yes, sorry, according to local code, seems like memory was not added
(was reverted memblock_find_in_range for some other reson while
porting), so I had only reserve. The issue is in another place,
so ignoe this patch, for now.

> > Fixes: a94e4f24ec83 ("MIPS: init: Drop boot_mem_map")
> > Signed-off-by: Ivan Khoronzhuk <ikhoronz@xxxxxxxxx>
> > ---
> > Based on linux-next/master
> >
> >  arch/mips/kernel/setup.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c
> > index 3785c72bc3bc..25e376ef2f2a 100644
> > --- a/arch/mips/kernel/setup.c
> > +++ b/arch/mips/kernel/setup.c
> > @@ -473,14 +473,15 @@ static void __init mips_parse_crashkernel(void)
> >  	crashk_res.end	 = crash_base + crash_size - 1;
> >  }
> >
> > -static void __init request_crashkernel(struct resource *res)
> > +static void __init request_crashkernel(void)
> >  {
> >  	int ret;
> >
> >  	if (crashk_res.start == crashk_res.end)
> >  		return;
> >
> > -	ret = request_resource(res, &crashk_res);
> > +	/* The crashk resource shoud be located in normal mem */
> > +	ret = insert_resource(&iomem_resource, &crashk_res);
> >  	if (!ret)
> >  		pr_info("Reserving %ldMB of memory at %ldMB for crashkernel\n",
> >  			(unsigned long)(resource_size(&crashk_res) >> 20),
> > @@ -734,8 +735,9 @@ static void __init resource_init(void)
> >  		request_resource(res, &code_resource);
> >  		request_resource(res, &data_resource);
> >  		request_resource(res, &bss_resource);
> > -		request_crashkernel(res);
> >  	}
> > +
> > +	request_crashkernel();
> >  }
> >
> >  #ifdef CONFIG_SMP
> > --
> > 2.23.1
>
> --
> Sincerely yours,
> Mike.

--
Regards,
Ivan Khoronzhuk