Re: [PATCH v3 2/2] mm: Introduce kernelcore=mirror option

[Andrew Morton <akpm@linux-foundation.org>]

On Wed,  9 Dec 2015 12:19:37 +0900 Taku Izumi <izumi.taku@jp.fujitsu.com> wrote:

> This patch extends existing "kernelcore" option and
> introduces kernelcore=mirror option. By specifying
> "mirror" instead of specifying the amount of memory,
> non-mirrored (non-reliable) region will be arranged
> into ZONE_MOVABLE.
> 
> v1 -> v2:
>  - Refine so that the following case also can be
>    handled properly:
> 
>  Node X:  |MMMMMM------MMMMMM--------|
>    (legend) M: mirrored  -: not mirrrored
> 
>  In this case, ZONE_NORMAL and ZONE_MOVABLE are
>  arranged like bellow:
> 
>  Node X:  |MMMMMM------MMMMMM--------|
>           |ooooooxxxxxxooooooxxxxxxxx| ZONE_NORMAL
>                 |ooooooxxxxxxoooooooo| ZONE_MOVABLE
>    (legend) o: present  x: absent
> 
> ...
>
>
> @@ -5507,6 +5569,36 @@ static void __init find_zone_movable_pfns_for_nodes(void)
>  	}
>  
>  	/*
> +	 * If kernelcore=mirror is specified, ignore movablecore option
> +	 */
> +	if (mirrored_kernelcore) {
> +		bool mem_below_4gb_not_mirrored = false;
> +
> +		for_each_memblock(memory, r) {
> +			if (memblock_is_mirror(r))
> +				continue;
> +
> +			nid = r->nid;
> +
> +			usable_startpfn = memblock_region_memory_base_pfn(r);
> +
> +			if (usable_startpfn < 0x100000) {
> +				mem_below_4gb_not_mirrored = true;
> +				continue;
> +			}
> +
> +			zone_movable_pfn[nid] = zone_movable_pfn[nid] ?
> +				min(usable_startpfn, zone_movable_pfn[nid]) :
> +				usable_startpfn;
> +		}
> +
> +		if (mem_below_4gb_not_mirrored)
> +			pr_warn("This configuration results in unmirrored kernel memory.");

This looks a bit strange.

What's the code actually doing?  Checking to see that all memory at
physical addresses < 0x100000000 is mirrored, I think?

If so, what's up with that hard-wired 0x100000?  That's not going to
work on PAGE_SIZE>4k machines, and this is generic code.

Also, I don't think the magical 0x100000000 is necessarily going to be
relevant for other architectures (presumably powerpc will be next...)

I guess this is all OK as an "initial implementation for x86_64 only"
for now, and this stuff will be changed later if/when another
architecture comes along.  However it would be nice to make things more
generic from the outset, to provide a guide for how things should be
implemented by other architectures.



Separately, what feedback does the user get about the success of the
kernelcore=mirror comment?  We didn't include an explicit printk which
displays the outcome, so how is the user to find out that it all worked
OK and that the kernel is now using mirrored memory?