[PATCH v3 2/3] x86: probe memory block size advisement value during mm init

Gregory Price posted 3 patches 1 month ago
There is a newer version of this series
[PATCH v3 2/3] x86: probe memory block size advisement value during mm init
Posted by Gregory Price 1 month ago
Systems with hotplug may provide an advisement value on what the
memblock size should be.  Probe this value when the rest of the
configuration values are considered.

The new heuristic is as follows

1) set_memory_block_size_order value if already set (cmdline param)
2) minimum block size if memory is less than large block limit
3) if no hotplug advice: Max block size if system is bare-metal,
   otherwise use end of memory alignment.
4) if hotplug advice: lesser of advice and end of memory alignment.

Suggested-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Gregory Price <gourry@gourry.net>
---
 arch/x86/mm/init_64.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index ff253648706f..93d669f467f7 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1452,13 +1452,17 @@ static unsigned long probe_memory_block_size(void)
 	}
 
 	/*
-	 * Use max block size to minimize overhead on bare metal, where
-	 * alignment for memory hotplug isn't a concern.
+	 * When hotplug alignment is not a concern, maximize blocksize
+	 * to minimize overhead. Otherwise, align to the lesser of advice
+	 * alignment and end of memory alignment.
 	 */
-	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
+	bz = memory_block_probe_max_size();
+	if (!bz) {
 		bz = MAX_BLOCK_SIZE;
-		goto done;
-	}
+		if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
+			goto done;
+	} else
+		bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);
 
 	/* Find the largest allowed block size that aligns to memory end */
 	for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
-- 
2.43.0
Re: [PATCH v3 2/3] x86: probe memory block size advisement value during mm init
Posted by David Hildenbrand 3 weeks, 6 days ago
On 22.10.24 23:34, Gregory Price wrote:
> Systems with hotplug may provide an advisement value on what the
> memblock size should be.  Probe this value when the rest of the
> configuration values are considered.
> 
> The new heuristic is as follows
> 
> 1) set_memory_block_size_order value if already set (cmdline param)
> 2) minimum block size if memory is less than large block limit
> 3) if no hotplug advice: Max block size if system is bare-metal,
>     otherwise use end of memory alignment.
> 4) if hotplug advice: lesser of advice and end of memory alignment.
> 
> Suggested-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Gregory Price <gourry@gourry.net>
> ---
>   arch/x86/mm/init_64.c | 14 +++++++++-----
>   1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index ff253648706f..93d669f467f7 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1452,13 +1452,17 @@ static unsigned long probe_memory_block_size(void)
>   	}
>   
>   	/*
> -	 * Use max block size to minimize overhead on bare metal, where
> -	 * alignment for memory hotplug isn't a concern.
> +	 * When hotplug alignment is not a concern, maximize blocksize
> +	 * to minimize overhead. Otherwise, align to the lesser of advice
> +	 * alignment and end of memory alignment.
>   	 */
> -	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
> +	bz = memory_block_probe_max_size();
> +	if (!bz) {
>   		bz = MAX_BLOCK_SIZE;
> -		goto done;
> -	}
> +		if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> +			goto done;
> +	} else
> +		bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);
>   
>   	/* Find the largest allowed block size that aligns to memory end */
>   	for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {


Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb
Re: [PATCH v3 2/3] x86: probe memory block size advisement value during mm init
Posted by Gregory Price 3 weeks, 6 days ago
On Tue, Oct 29, 2024 at 01:40:38PM +0100, David Hildenbrand wrote:
> On 22.10.24 23:34, Gregory Price wrote:
> > Systems with hotplug may provide an advisement value on what the
> > memblock size should be.  Probe this value when the rest of the
> > configuration values are considered.
> > 
> > The new heuristic is as follows
> > 
> > 1) set_memory_block_size_order value if already set (cmdline param)
> > 2) minimum block size if memory is less than large block limit
> > 3) if no hotplug advice: Max block size if system is bare-metal,
> >     otherwise use end of memory alignment.
> > 4) if hotplug advice: lesser of advice and end of memory alignment.
> > 
> > Suggested-by: David Hildenbrand <david@redhat.com>
> > Signed-off-by: Gregory Price <gourry@gourry.net>
> > ---
> >   arch/x86/mm/init_64.c | 14 +++++++++-----
> >   1 file changed, 9 insertions(+), 5 deletions(-)
> > 
> > diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> > index ff253648706f..93d669f467f7 100644
> > --- a/arch/x86/mm/init_64.c
> > +++ b/arch/x86/mm/init_64.c
> > @@ -1452,13 +1452,17 @@ static unsigned long probe_memory_block_size(void)
> >   	}
> >   	/*
> > -	 * Use max block size to minimize overhead on bare metal, where
> > -	 * alignment for memory hotplug isn't a concern.
> > +	 * When hotplug alignment is not a concern, maximize blocksize
> > +	 * to minimize overhead. Otherwise, align to the lesser of advice
> > +	 * alignment and end of memory alignment.
> >   	 */
> > -	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
> > +	bz = memory_block_probe_max_size();
> > +	if (!bz) {
> >   		bz = MAX_BLOCK_SIZE;
> > -		goto done;
> > -	}
> > +		if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
> > +			goto done;
> > +	} else
> > +		bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);
> >   	/* Find the largest allowed block size that aligns to memory end */
> >   	for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
        ^^^^^^^^^^^^^^^^^^^^^^^^
> 
> 
> Acked-by: David Hildenbrand <david@redhat.com>

Will pick this up but wanted to point out the silly bug above.
This version completely ignores the advise lol.

Changing to below

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 93d669f467f7..01876629f21f 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -1465,7 +1465,7 @@ static unsigned long probe_memory_block_size(void)
                bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);

        /* Find the largest allowed block size that aligns to memory end */
-       for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
+       for (; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
                if (IS_ALIGNED(boot_mem_end, bz))
                        break;
        }

> 
> -- 
> Cheers,
> 
> David / dhildenb
>
Re: [PATCH v3 2/3] x86: probe memory block size advisement value during mm init
Posted by David Hildenbrand 3 weeks, 6 days ago
On 29.10.24 14:48, Gregory Price wrote:
> On Tue, Oct 29, 2024 at 01:40:38PM +0100, David Hildenbrand wrote:
>> On 22.10.24 23:34, Gregory Price wrote:
>>> Systems with hotplug may provide an advisement value on what the
>>> memblock size should be.  Probe this value when the rest of the
>>> configuration values are considered.
>>>
>>> The new heuristic is as follows
>>>
>>> 1) set_memory_block_size_order value if already set (cmdline param)
>>> 2) minimum block size if memory is less than large block limit
>>> 3) if no hotplug advice: Max block size if system is bare-metal,
>>>      otherwise use end of memory alignment.
>>> 4) if hotplug advice: lesser of advice and end of memory alignment.
>>>
>>> Suggested-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Gregory Price <gourry@gourry.net>
>>> ---
>>>    arch/x86/mm/init_64.c | 14 +++++++++-----
>>>    1 file changed, 9 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
>>> index ff253648706f..93d669f467f7 100644
>>> --- a/arch/x86/mm/init_64.c
>>> +++ b/arch/x86/mm/init_64.c
>>> @@ -1452,13 +1452,17 @@ static unsigned long probe_memory_block_size(void)
>>>    	}
>>>    	/*
>>> -	 * Use max block size to minimize overhead on bare metal, where
>>> -	 * alignment for memory hotplug isn't a concern.
>>> +	 * When hotplug alignment is not a concern, maximize blocksize
>>> +	 * to minimize overhead. Otherwise, align to the lesser of advice
>>> +	 * alignment and end of memory alignment.
>>>    	 */
>>> -	if (!boot_cpu_has(X86_FEATURE_HYPERVISOR)) {
>>> +	bz = memory_block_probe_max_size();
>>> +	if (!bz) {
>>>    		bz = MAX_BLOCK_SIZE;
>>> -		goto done;
>>> -	}
>>> +		if (!boot_cpu_has(X86_FEATURE_HYPERVISOR))
>>> +			goto done;
>>> +	} else
>>> +		bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);
>>>    	/* Find the largest allowed block size that aligns to memory end */
>>>    	for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
>          ^^^^^^^^^^^^^^^^^^^^^^^^
>>
>>
>> Acked-by: David Hildenbrand <david@redhat.com>
> 
> Will pick this up but wanted to point out the silly bug above.
> This version completely ignores the advise lol.
> 
> Changing to below
> 
> diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
> index 93d669f467f7..01876629f21f 100644
> --- a/arch/x86/mm/init_64.c
> +++ b/arch/x86/mm/init_64.c
> @@ -1465,7 +1465,7 @@ static unsigned long probe_memory_block_size(void)
>                  bz = max(min(bz, MAX_BLOCK_SIZE), MIN_MEMORY_BLOCK_SIZE);
> 
>          /* Find the largest allowed block size that aligns to memory end */
> -       for (bz = MAX_BLOCK_SIZE; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
> +       for (; bz > MIN_MEMORY_BLOCK_SIZE; bz >>= 1) {
>                  if (IS_ALIGNED(boot_mem_end, bz))
>                          break;

Heh, yes, I think I suggested that in my quick draft.

-- 
Cheers,

David / dhildenb