kho: fixes and cleanups

[PATCH 1/3] kho: allow scratch areas with zero size

Posted by Mike Rapoport 1 month, 3 weeks ago

From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>

Parsing of kho_scratch parameter treats zero size as an invalid value,
although it should be fine for user to request zero sized scratch area
for some types if scratch memory, when for example there is no need to
create scratch area in the low memory.

Treat zero as a valid value for a scratch area size but reject
kho_scratch parameter that defines no scratch memory at all.

Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 kernel/kexec_handover.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
index e49743ae52c5..c6ac5a5e51cb 100644
--- a/kernel/kexec_handover.c
+++ b/kernel/kexec_handover.c
@@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
 {
 	size_t len;
 	unsigned long sizes[3];
+	size_t total_size = 0;
 	int i;
 
 	if (!p)
@@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
 		}
 
 		sizes[i] = memparse(p, &endp);
-		if (!sizes[i] || endp == p)
+		if (endp == p)
 			return -EINVAL;
 		p = endp;
+		total_size += sizes[i];
 	}
 
+	if (!total_size)
+		return -EINVAL;
+
 	scratch_size_lowmem = sizes[0];
 	scratch_size_global = sizes[1];
 	scratch_size_pernode = sizes[2];
-- 
2.47.2

Re: [PATCH 1/3] kho: allow scratch areas with zero size

Posted by Pratyush Yadav 1 month, 3 weeks ago

On Mon, Aug 11 2025, Mike Rapoport wrote:

> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>
> Parsing of kho_scratch parameter treats zero size as an invalid value,
> although it should be fine for user to request zero sized scratch area
> for some types if scratch memory, when for example there is no need to
> create scratch area in the low memory.

Can the system boot with 0 per-node memory? If not, then perhaps we
should only allow lowmem scratch to be zero?

>
> Treat zero as a valid value for a scratch area size but reject
> kho_scratch parameter that defines no scratch memory at all.
>
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> ---
>  kernel/kexec_handover.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> index e49743ae52c5..c6ac5a5e51cb 100644
> --- a/kernel/kexec_handover.c
> +++ b/kernel/kexec_handover.c
> @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
>  {
>  	size_t len;
>  	unsigned long sizes[3];
> +	size_t total_size = 0;
>  	int i;
>  
>  	if (!p)
> @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
>  		}
>  
>  		sizes[i] = memparse(p, &endp);
> -		if (!sizes[i] || endp == p)
> +		if (endp == p)
>  			return -EINVAL;
>  		p = endp;
> +		total_size += sizes[i];
>  	}
>  
> +	if (!total_size)
> +		return -EINVAL;
> +

Looks good. BTW, unrelated to this patch, but should we also check that
p == '\0' here to make sure the whole argument was consumed?


>  	scratch_size_lowmem = sizes[0];
>  	scratch_size_global = sizes[1];
>  	scratch_size_pernode = sizes[2];

-- 
Regards,
Pratyush Yadav

Re: [PATCH 1/3] kho: allow scratch areas with zero size

Posted by Mike Rapoport 1 month, 3 weeks ago

On Wed, Aug 13, 2025 at 03:45:29PM +0200, Pratyush Yadav wrote:
> On Mon, Aug 11 2025, Mike Rapoport wrote:
> 
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> >
> > Parsing of kho_scratch parameter treats zero size as an invalid value,
> > although it should be fine for user to request zero sized scratch area
> > for some types if scratch memory, when for example there is no need to
> > create scratch area in the low memory.
> 
> Can the system boot with 0 per-node memory? If not, then perhaps we
> should only allow lowmem scratch to be zero?

In most cases yes because most of boot time allocations have fallback to
"any node".
And there's also an option to omit the "global" scratch and boot with only
per-node scratch areas, so I'd keep the possibility of setting any of these
to 0.

> > Treat zero as a valid value for a scratch area size but reject
> > kho_scratch parameter that defines no scratch memory at all.
> >
> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
> > ---
> >  kernel/kexec_handover.c | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
> > index e49743ae52c5..c6ac5a5e51cb 100644
> > --- a/kernel/kexec_handover.c
> > +++ b/kernel/kexec_handover.c
> > @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
> >  {
> >  	size_t len;
> >  	unsigned long sizes[3];
> > +	size_t total_size = 0;
> >  	int i;
> >  
> >  	if (!p)
> > @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
> >  		}
> >  
> >  		sizes[i] = memparse(p, &endp);
> > -		if (!sizes[i] || endp == p)
> > +		if (endp == p)
> >  			return -EINVAL;
> >  		p = endp;
> > +		total_size += sizes[i];
> >  	}
> >  
> > +	if (!total_size)
> > +		return -EINVAL;
> > +
> 
> Looks good. BTW, unrelated to this patch, but should we also check that
> p == '\0' here to make sure the whole argument was consumed?

Care to send a patch? ;-) 
 
> >  	scratch_size_lowmem = sizes[0];
> >  	scratch_size_global = sizes[1];
> >  	scratch_size_pernode = sizes[2];
> 
> -- 
> Regards,
> Pratyush Yadav

-- 
Sincerely yours,
Mike.

Re: [PATCH 1/3] kho: allow scratch areas with zero size

Posted by Pratyush Yadav 1 month, 3 weeks ago

On Wed, Aug 13 2025, Mike Rapoport wrote:

> On Wed, Aug 13, 2025 at 03:45:29PM +0200, Pratyush Yadav wrote:
>> On Mon, Aug 11 2025, Mike Rapoport wrote:
>> 
>> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>> >
>> > Parsing of kho_scratch parameter treats zero size as an invalid value,
>> > although it should be fine for user to request zero sized scratch area
>> > for some types if scratch memory, when for example there is no need to
>> > create scratch area in the low memory.
>> 
>> Can the system boot with 0 per-node memory? If not, then perhaps we
>> should only allow lowmem scratch to be zero?
>
> In most cases yes because most of boot time allocations have fallback to
> "any node".
> And there's also an option to omit the "global" scratch and boot with only
> per-node scratch areas, so I'd keep the possibility of setting any of these
> to 0.

Makes sense. In that case,

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

>
>> > Treat zero as a valid value for a scratch area size but reject
>> > kho_scratch parameter that defines no scratch memory at all.
>> >
>> > Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
>> > ---
>> >  kernel/kexec_handover.c | 7 ++++++-
>> >  1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/kernel/kexec_handover.c b/kernel/kexec_handover.c
>> > index e49743ae52c5..c6ac5a5e51cb 100644
>> > --- a/kernel/kexec_handover.c
>> > +++ b/kernel/kexec_handover.c
>> > @@ -385,6 +385,7 @@ static int __init kho_parse_scratch_size(char *p)
>> >  {
>> >  	size_t len;
>> >  	unsigned long sizes[3];
>> > +	size_t total_size = 0;
>> >  	int i;
>> >  
>> >  	if (!p)
>> > @@ -421,11 +422,15 @@ static int __init kho_parse_scratch_size(char *p)
>> >  		}
>> >  
>> >  		sizes[i] = memparse(p, &endp);
>> > -		if (!sizes[i] || endp == p)
>> > +		if (endp == p)
>> >  			return -EINVAL;
>> >  		p = endp;
>> > +		total_size += sizes[i];
>> >  	}
>> >  
>> > +	if (!total_size)
>> > +		return -EINVAL;
>> > +
>> 
>> Looks good. BTW, unrelated to this patch, but should we also check that
>> p == '\0' here to make sure the whole argument was consumed?
>
> Care to send a patch? ;-) 

Will do :-)

>  
>> >  	scratch_size_lowmem = sizes[0];
>> >  	scratch_size_global = sizes[1];
>> >  	scratch_size_pernode = sizes[2];
>> 
>> -- 
>> Regards,
>> Pratyush Yadav

-- 
Regards,
Pratyush Yadav