diff mbox

[v3,5/5] Use 2GB memory block size on large x86-64 systems

Message ID 1414915813-2462-5-git-send-email-daniel@numascale.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Daniel J Blueman Nov. 2, 2014, 8:10 a.m. UTC 
On larger x64-64 systems, use a 2GB memory block size to reduce sysfs
entry creation time by 16x. Large is defined as 64GB or more memory.

Signed-off-by: Daniel J Blueman <daniel@numascale.com>
---
 arch/x86/mm/init_64.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Thomas Gleixner Nov. 3, 2014, 7:38 p.m. UTC | #1 
On Sun, 2 Nov 2014, Daniel J Blueman wrote:

> On larger x64-64 systems, use a 2GB memory block size to reduce sysfs
> entry creation time by 16x. Large is defined as 64GB or more memory.

This changelog sucks.

It neither tells which sysfs entries are meant nor does it explain
what the actual effect of this change is aside of speeding up some
random sysfs thingy.

> @@ -1247,9 +1246,9 @@ static unsigned long probe_memory_block_size(void)
>  	/* start from 2g */
>  	unsigned long bz = 1UL<<31;
>  
> -#ifdef CONFIG_X86_UV
> -	if (is_uv_system()) {
> -		printk(KERN_INFO "UV: memory block size 2GB\n");
> +#ifdef CONFIG_X86_64

And this brainless 's/CONFIG_X86_UV/CONFIG_X86_64/' sucks even
more. I'm sure you can figure out the WHY yourself.

> +	if (totalram_pages >= (64ULL << (30 - PAGE_SHIFT))) {
> +		pr_info("Using 2GB memory block size for large-memory system\n");
>  		return 2UL * 1024 * 1024 * 1024;
>  	}
>  #endif

Thanks,

	tglx

 
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel J Blueman Nov. 3, 2014, 11:15 p.m. UTC | #2 
On 11/04/2014 03:38 AM, Thomas Gleixner wrote:
> On Sun, 2 Nov 2014, Daniel J Blueman wrote:
>
>> On larger x64-64 systems, use a 2GB memory block size to reduce sysfs
>> entry creation time by 16x. Large is defined as 64GB or more memory.
>
> This changelog sucks.
>
> It neither tells which sysfs entries are meant nor does it explain
> what the actual effect of this change is aside of speeding up some
> random sysfs thingy.

How about this?

On large-memory systems of 64GB or more with memory hot-plug enabled, 
use a 2GB memory block size. Eg with 64GB memory, this reduces the 
number of directories in /sys/devices/system/memory from 512 to 32, 
making it more manageable, and reducing the creation time accordingly.

>> @@ -1247,9 +1246,9 @@ static unsigned long probe_memory_block_size(void)
>>   	/* start from 2g */
>>   	unsigned long bz = 1UL<<31;
>>
>> -#ifdef CONFIG_X86_UV
>> -	if (is_uv_system()) {
>> -		printk(KERN_INFO "UV: memory block size 2GB\n");
>> +#ifdef CONFIG_X86_64
>
> And this brainless 's/CONFIG_X86_UV/CONFIG_X86_64/' sucks even
> more. I'm sure you can figure out the WHY yourself.

The benefit of this is applicable to other architectures. I'm unable to 
test the change, but if you agree it's conservative enough, I'll drop 
the ifdef?

Thanks,
   Daniel
Thomas Gleixner Nov. 3, 2014, 11:36 p.m. UTC | #3 
On Tue, 4 Nov 2014, Daniel J Blueman wrote:

> On 11/04/2014 03:38 AM, Thomas Gleixner wrote:
> > On Sun, 2 Nov 2014, Daniel J Blueman wrote:
> > 
> > > On larger x64-64 systems, use a 2GB memory block size to reduce sysfs
> > > entry creation time by 16x. Large is defined as 64GB or more memory.
> > 
> > This changelog sucks.
> > 
> > It neither tells which sysfs entries are meant nor does it explain
> > what the actual effect of this change is aside of speeding up some
> > random sysfs thingy.
> 
> How about this?
> 
> On large-memory systems of 64GB or more with memory hot-plug enabled, use a
> 2GB memory block size. Eg with 64GB memory, this reduces the number of
> directories in /sys/devices/system/memory from 512 to 32, making it more
> manageable, and reducing the creation time accordingly.

It still does not tell what the downside is of this and why you think
it does not matter.
 
> > > @@ -1247,9 +1246,9 @@ static unsigned long probe_memory_block_size(void)
> > >   	/* start from 2g */
> > >   	unsigned long bz = 1UL<<31;
> > > 
> > > -#ifdef CONFIG_X86_UV
> > > -	if (is_uv_system()) {
> > > -		printk(KERN_INFO "UV: memory block size 2GB\n");
> > > +#ifdef CONFIG_X86_64
> > 
> > And this brainless 's/CONFIG_X86_UV/CONFIG_X86_64/' sucks even
> > more. I'm sure you can figure out the WHY yourself.
> 
> The benefit of this is applicable to other architectures. I'm unable to test
> the change, but if you agree it's conservative enough, I'll drop the ifdef?

Which other architectures? Care to turn on your brain before replying?

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel J Blueman Nov. 4, 2014, 7:30 a.m. UTC | #4 
On 11/04/2014 07:36 AM, Thomas Gleixner wrote:
> On Tue, 4 Nov 2014, Daniel J Blueman wrote:
>
>> On 11/04/2014 03:38 AM, Thomas Gleixner wrote:
>>> On Sun, 2 Nov 2014, Daniel J Blueman wrote:
>>>
>>>> On larger x64-64 systems, use a 2GB memory block size to reduce sysfs
>>>> entry creation time by 16x. Large is defined as 64GB or more memory.
>>>
>>> This changelog sucks.
>>>
>>> It neither tells which sysfs entries are meant nor does it explain
>>> what the actual effect of this change is aside of speeding up some
>>> random sysfs thingy.
>>
>> How about this?
>>
>> On large-memory systems of 64GB or more with memory hot-plug enabled, use a
>> 2GB memory block size. Eg with 64GB memory, this reduces the number of
>> directories in /sys/devices/system/memory from 512 to 32, making it more
>> manageable, and reducing the creation time accordingly.
>
> It still does not tell what the downside is of this and why you think
> it does not matter.

Yes, let's make it explicit:

On large-memory systems of 64GB or more with memory hot-plug enabled, 
use a 2GB memory block size. Eg with 64GB memory, this reduces the 
number of directories in /sys/devices/system/memory from 512 to 32, 
making it more manageable, and reducing the creation time accordingly.

This caveat is that the memory can't be offlined (for hotplug or 
otherwise) with finer 128MB granularity, but this is unimportant due to 
the high memory densities generally used with such large-memory systems, 
where eg a single DIMM is the order of 16GB.

>>>> @@ -1247,9 +1246,9 @@ static unsigned long probe_memory_block_size(void)
>>>>    	/* start from 2g */
>>>>    	unsigned long bz = 1UL<<31;
>>>>
>>>> -#ifdef CONFIG_X86_UV
>>>> -	if (is_uv_system()) {
>>>> -		printk(KERN_INFO "UV: memory block size 2GB\n");
>>>> +#ifdef CONFIG_X86_64
>>>
>>> And this brainless 's/CONFIG_X86_UV/CONFIG_X86_64/' sucks even
>>> more. I'm sure you can figure out the WHY yourself.
>>
>> The benefit of this is applicable to other architectures. I'm unable to test
>> the change, but if you agree it's conservative enough, I'll drop the ifdef?
>
> Which other architectures? Care to turn on your brain before replying?

Clearly 64-bit architectures, including X86, MIPS, PARISC, SPARC, 
AArch64, ia64, however, I must be missing something, as a 
sizeof(long)/CONFIG_64BIT check would be redundant if we agree to drop 
the ifdef, as we're already checking the number of physical pages, which 
is bounded by the same limits.

Thanks,
   Daniel
Thomas Gleixner Nov. 4, 2014, 7:50 a.m. UTC | #5 
On Tue, 4 Nov 2014, Daniel J Blueman wrote:
> On 11/04/2014 07:36 AM, Thomas Gleixner wrote:
> > On Tue, 4 Nov 2014, Daniel J Blueman wrote:
> > > > > @@ -1247,9 +1246,9 @@ static unsigned long
> > > > > probe_memory_block_size(void)
> > > > >    	/* start from 2g */
> > > > >    	unsigned long bz = 1UL<<31;
> > > > > 
> > > > > -#ifdef CONFIG_X86_UV
> > > > > -	if (is_uv_system()) {
> > > > > -		printk(KERN_INFO "UV: memory block size 2GB\n");
> > > > > +#ifdef CONFIG_X86_64
> > > > 
> > > > And this brainless 's/CONFIG_X86_UV/CONFIG_X86_64/' sucks even
> > > > more. I'm sure you can figure out the WHY yourself.
> > > 
> > > The benefit of this is applicable to other architectures. I'm unable to
> > > test
> > > the change, but if you agree it's conservative enough, I'll drop the
> > > ifdef?
> > 
> > Which other architectures? Care to turn on your brain before replying?
> 
> Clearly 64-bit architectures, including X86, MIPS, PARISC, SPARC, AArch64,
> ia64,

ROTFL

> however, I must be missing something, as a sizeof(long)/CONFIG_64BIT
> check would be redundant if we agree to drop the ifdef, as we're already
> checking the number of physical pages, which is bounded by the same limits.

# diffstat $this_patch

should precicely tell you what you're missing.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/x86/mm/init_64.c b/arch/x86/mm/init_64.c
index 4cb8763..6002e80 100644
--- a/arch/x86/mm/init_64.c
+++ b/arch/x86/mm/init_64.c
@@ -52,7 +52,6 @@ 
 #include <asm/numa.h>
 #include <asm/cacheflush.h>
 #include <asm/init.h>
-#include <asm/uv/uv.h>
 #include <asm/setup.h>
 
 #include "mm_internal.h"
@@ -1247,9 +1246,9 @@  static unsigned long probe_memory_block_size(void)
 	/* start from 2g */
 	unsigned long bz = 1UL<<31;
 
-#ifdef CONFIG_X86_UV
-	if (is_uv_system()) {
-		printk(KERN_INFO "UV: memory block size 2GB\n");
+#ifdef CONFIG_X86_64
+	if (totalram_pages >= (64ULL << (30 - PAGE_SHIFT))) {
+		pr_info("Using 2GB memory block size for large-memory system\n");
 		return 2UL * 1024 * 1024 * 1024;
 	}
 #endif