Skip to contentSkip to conversation

1/5 in support of hot-add memory x86_64 nodes_add cleanup

PATCH In review keith mannthey 2 participants 4 messages Jul 29, 2006

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 1/5 in support of hot-add memory x86_64 nodes_add cleanup

This patch cleans up the srat.c file with respects to the 2 hot-add
paths available. It creates Kconfig options to represent the desired functionality and implements their usage.

The current code already saves the information need (NUMA locality of
hot-add memory ranges) for MEMORY_HOTPLUG in the RESERVE_MEMORY path. I amend the memory parsing code to support the needs of both and save the nodes_add data for use at runtime.

Built against 2.6.18-rc2

arch/x86_64/Kconfig +11 -0
diff -urN orig/arch/x86_64/Kconfig work/arch/x86_64/Kconfig
@@ -349,6 +349,17 @@
 
 source "mm/Kconfig"
 
+config RESERVE_HOTADD
+	bool "Reserve based hot-add memory" 
+	depends on DISCONTIGMEM && X86_64_ACPI_NUMA
+	default n
+	help
+	  This allows a reserved based approach to hot add memory.
+	  Select this option if your hardware supports hot-add memroy 
+	  via ACPI and the SRAT table.  You will need to still build in 
+	  the ACPI hot plug memory driver to do the actual add. 
+
+
 config HAVE_ARCH_EARLY_PFN_TO_NID
 	def_bool y
 	depends on NUMA
arch/x86_64/mm/srat.c +38 -29

154 lines not on this page · raw message

#1 lore raw

If you add attachments (which are harder to review because they cannot be quoted) put at least a copy of the description into the attachment and make them inline.

One thing that's unclear to me is why you make the reserve hotadd Kconfig user visible (and why put it into Kconfig at all). I don't think it should be user visible.

Also most of the patch seems to be renaming a variable which seems somewhat pointless?

-Andi

Quoted 12 lines · keith mannthey

This patch cleans up the srat.c file with respects to the 2 hot-add
paths available. It creates Kconfig options to represent the desired functionality and implements their usage.

The current code already saves the information need (NUMA locality of
hot-add memory ranges) for MEMORY_HOTPLUG in the RESERVE_MEMORY path. I amend the memory parsing code to support the needs of both and save the nodes_add data for use at runtime.

Built against 2.6.18-rc2

#2 lore raw
Quoted 3 lines · Andi Kleen

If you add attachments (which are harder to review because they cannot be quoted) put at least a copy of the description into the attachment and make them inline.

Sorry I will do this if the future.

Quoted 4 lines

One thing that's unclear to me is why you make the reserve hotadd Kconfig user visible (and why put it into Kconfig at all). I don't think it should be user visible.

It was suggested to me in an earlier patch set to move RESERVE_HOTADD to Kconfig. I can make it a non-user option. It seems that MEMORY_HOTPLUG is a user options so I figured why not make RESERVE one as well. Also I don't think people should use reserve with sparsmem.

Quoted 2 lines

Also most of the patch seems to be renaming a variable which seems somewhat pointless?

srat.c as it stands is very reserve specific (understandably).
Changing found_add_area to reserve_add_area is extra change help the var names be more clear with the sharing of the code path as to what is happening. reserve_add_area is used as a switch for the reserve path. Since the logic of the code was changing I thought it would be a good time to change the name to help the clarity of the code.

If you see this as a waste/unneeded I will revert the name change.

Thanks for the comments,
Keith

#3 lore raw
Quoted 3 lines

It was suggested to me in an earlier patch set to move RESERVE_HOTADD to Kconfig.

It doesn't make any sense to me.

Quoted 3 lines

I can make it a non-user option. It seems that MEMORY_HOTPLUG is a user options so I figured why not make RESERVE one as well. Also I don't think people should use reserve with sparsmem.

There should be a single high level Kconfig option for each feature.

-Andi