Re: [PATCH v2 15/18] kho: extend scratch

[Pratyush Yadav <pratyush@kernel.org>]

On Sun, Jun 14 2026, Mike Rapoport wrote:

> On Fri, 05 Jun 2026 20:34:48 +0200, Pratyush Yadav <pratyush@kernel.org> wrote:
>> diff --git a/kernel/liveupdate/kexec_handover.c b/kernel/liveupdate/kexec_handover.c
>> index af22086ca2d6..8540608b8602 100644
>> --- a/kernel/liveupdate/kexec_handover.c
>> +++ b/kernel/liveupdate/kexec_handover.c
>> @@ -869,6 +886,119 @@ static void __init kho_reserve_scratch(void)
>>  	kho_enable = false;
>>  }
>>  
>> +#define KHO_EXT_SHIFT 30 /* 1 GiB */
>
> Please add a comment why exactly 1 Gib.
> I'd also define a size macro as SZ_1G and make shift const_ilog2(SIZE)

ACK.

>
>> +
>> +static int __init kho_ext_walk_key(unsigned long key, void *data)
>
> Maybe _leaf? No strong feelings though

Sure.

>
>> +{
>> +	struct kho_radix_tree *tree = data;
>
> Would be nice to say which tree in the variable name ;)

How about preserved_mem_map?

>
>> [ ... skip 15 lines ... ]
>> +	return 0;
>> +}
>> +
>> +static int __init kho_ext_walk_node(phys_addr_t phys, void *data)
>> +{
>> +	struct kho_radix_tree *tree = data;
>
> Ditto
>
>> [ ... skip 15 lines ... ]
>> +
>> +	*prev_end = start + (1UL << KHO_EXT_SHIFT);
>> +	return 0;
>> +}
>> +
>> +/**
>
> I don't think we expose statics as kernel-doc somewhere, so this
> probably shouldn't be a kernel-doc comment

This is a leftover from the previous version, where this was external.

I think the documentation is still worthwhile though, so I suppose I'll
turn it into a normal comment by removing the /**.

>
>> + * kho_extend_scratch - Extend the scratch regions
>> + *
>> + * The KHO radix tree mixes both physical address and order into a single key.
>
> Here it's rather the preserved memory map radix tree or something like
> that.

Sure, will update.

>
>> + * This makes it hard to look for free ranges directly. This function first
>> + * walks the radix tree and digests it down into another radix tree, whose keys
>
> Here as well, maybe don't even mention radix to make it shorter, the
> important part is that we wakk the preserved memory map and create a
> radix tree that identifies blocks around the preserved memory.

ACK.

>
>> + * identify blocks of KHO_EXT_SHIFT which contain preserved memory.
>> + *
>> + * Then it walks the digested radix tree and marks everything that doesn't have
>> + * preserved memory as scratch.
>> + *
>> + * NOTE: This function allocates memory so it should be called when scratch has
>> + * available space.
>> + *
>> + * NOTE: The pages of the KHO radix tree tables are not marked as preserved in
>
> ^ preserved memory map radix tree :)

ACK.

>
>> + * the KHO tree. But they are expected to remain untouched until the tree is
>> + * fully parsed. So this function also considers them to be "preserved memory"
>> + * and marks their blocks as busy.
>> + */
>> +static void __init kho_extend_scratch(void)
>> +{
>> +	const struct kho_radix_walk_cb kho_cb = {
>> +		.leaf = kho_ext_walk_key,
>> +		.node = kho_ext_walk_node,
>> +	};
>> +	const struct kho_radix_walk_cb ext_cb = {
>> +		.leaf = kho_ext_mark_scratch,
>> +	};
>> +	struct kho_radix_tree radix;
>
> sashiko says:
>
>   Is it possible for the radix variable to contain uninitialized stack memory
>   here?
>   If radix is uninitialized, tree->root might contain garbage data when passed
>   to kho_radix_init_tree()
>
> and I agree :)
>
> This should be
>
> 	struct kho_radix_tree radix = { 0 };

Ugh, right. But at the same time, it is odd for an initialization
function to expect an initialized object. Perhaps I should move the
kho_radix_init_tree() from kho_mem_retrieve() to
kho_memory_init_early(). Then kho_extend_scratch() won't have to do the
init at all and I can remove the if (tree->root) check from
kho_radix_init_tree().

-- 
Regards,
Pratyush Yadav