Re: [PATCH] kho: always print scratch sizes

[Pasha Tatashin <pasha.tatashin@soleen.com>]

On 05-19 17:53, Pratyush Yadav wrote:
> From: "Pratyush Yadav (Google)" <pratyush@kernel.org>
> 
> Currently, KHO only prints scratch sizes when the size is specified
> by the command line. It prints nothing when the size is calculated
> at runtime by counting RSRV_KERN.
> 
> These prints are completely useless. When the user specified the size on
> the command line, they already know how large the scratch areas are. And
> that information is readily available by looking at the command line. It
> is a lot more useful to know the sizes that are calculated at runtime
> since they can vary by kernel version. Plus, if KHO fails to allocate
> the scratch areas, the calculated scratch sizes are not available via
> debugfs.
> 
> Print the scratch sizes in kho_reserve_scratch(). Do it before making
> the allocations. The size can provide hints for the failure reason.
> 
> Signed-off-by: Pratyush Yadav (Google) <pratyush@kernel.org>
> ---
>  kernel/liveupdate/kexec_handover.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
> index 33fcf848ef95..b030f53f2235 100644
> --- a/kernel/liveupdate/kexec_handover.c
> +++ b/kernel/liveupdate/kexec_handover.c
> @@ -621,11 +621,6 @@ static int __init kho_parse_scratch_size(char *p)
>  	scratch_size_pernode = sizes[2];
>  	scratch_scale = 0;
>  
> -	pr_notice("scratch areas: lowmem: %lluMiB global: %lluMiB pernode: %lldMiB\n",
> -		  (u64)(scratch_size_lowmem >> 20),
> -		  (u64)(scratch_size_global >> 20),
> -		  (u64)(scratch_size_pernode >> 20));
> -
>  	return 0;
>  }
>  early_param("kho_scratch", kho_parse_scratch_size);
> @@ -691,6 +686,10 @@ static void __init kho_reserve_scratch(void)
>  		goto err_disable_kho;
>  	}
>  
> +	pr_notice("scratch areas: lowmem: %lluMiB global: %lluMiB\n",
> +		  (u64)(scratch_size_lowmem >> 20),
> +		  (u64)(scratch_size_global >> 20));
> +
>  	/*
>  	 * reserve scratch area in low memory for lowmem allocations in the
>  	 * next kernel
> @@ -725,6 +724,9 @@ static void __init kho_reserve_scratch(void)
>  	 */
>  	for_each_node_state(nid, N_MEMORY) {
>  		size = scratch_size_node(nid);
> +
> +		pr_notice("scratch_areas: nid %d: %lluMiB\n", nid, size >> 20);

Please use a space instead of an underscore to match the other string: 
"scratch areas". Keeping the prefix identical makes log parsing and 
filtering much cleaner.

Unrelated to this patch—this is something I brought up to Pratyush off-list, 
but I really dislike the 'KHO scratch' terminology.

First, it isn't actually 'scratch' space, as the kernel relies on it to 
allocate crucial early boot memory, which it then uses permanently. 
Second, we cannot preserve anything within this specific region. 

Down the line, I'd like to rename this to something more descriptive that clearly 
conveys both its importance to early boot and its non-preservable nature. For 
now, the print adjustments themselves look good.

I do not know it should be, but something like kho_bootstrap area sounds 
better than kho_sratch area.

> +
>  		addr = memblock_alloc_range_nid(size, CMA_MIN_ALIGNMENT_BYTES,
>  						0, MEMBLOCK_ALLOC_ACCESSIBLE,
>  						nid, true);
> 
> base-commit: 34e8f02817e31826e76bb2ded48bf28fe921f20b
> -- 
> 2.54.0.563.g4f69b47b94-goog
>