Re: [PATCH v4 2/4] x86: Store memory ranges globally used for crash kernel to boot into

[WANG Chao <chaowang@redhat.com>]

On 03/28/14 at 11:24am, Dave Young wrote:
> >  
> > +static void exclude_ram(struct memory_range *mr, int *nr_mr)
> > +{
> > +	int ranges, i, j, m;
> > +
> > +	ranges = *nr_mr;
> > +	for (i = 0, j = 0; i < ranges; i++) {
> > +		if (mr[j].type == RANGE_RAM) {
> > +			dbgprintf("Remove RAM %016llx-%016llxx: (%d)\n", mr[j].start, mr[j].end, mr[j].type);
> > +			for (m = j; m < *nr_mr; m++)
> > +				mr[m] = mr[m+1];
> > +			(*nr_mr)--;
> > +		} else {
> > +			j++;
> > +		}
> > +	}
> > +
> > +	dbgprint_mem_range("After remove RAM", mr, *nr_mr);
> > +}
> 
> This is probably not necessary, what I understand you are doing is below:
> 
> get_crash_memory_ranges()
>  -> collect all SYSTEM_RAM, ACPI, ACPI_NVS ranges, exclude crash reserved ranges.
>  -> the system ram ranges are used to create elf header
>  -> the ACPI, ACPI_NVS ranges are used by cmdline_add_memmap_acpi etc.

Yes.

> 
> memmap_p
>  -> contains all the crash reserved ranges
>  -> to be used by cmdline_add_memmap

There's no memmap_p. I'll reuse crash_memory_ranges structure to store
crash reserved ranges, ACPI and ACPI_NVS ranges. So after building ELF
headers for 1st kernel memory ranges, all I have to do is exclude the
SYSTEM_RAM and add crash_reserved to crash_memory_ranges. And then
crash_memory_ranges can be used as 2nd kernel memory ranges.

> 
> The several memory ranges are twisted and somehow the funcions are duplicate.

These functions look like similar but each does serve for different purpose.

> 
> So how about just keep one memory ranges array which contains all the ranges which
> include system_ram, acpi, acpi_nvs, crash_reserved range.
> 
> In the crashdump-elf.c the function for creating elf headers will check the
> range type, it will just skip the range which is not ram.

We don't build EFL headers for crash_reserved ranges. So we can't store
system ram and crash_reserved together before building EFL header.

> 
> Ditto for other functions they can also just select what range type they need instead
> of creating these different arrays which is confusing.
> 
> Thanks
> Dave

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec