Re: [PATCH 5/5] SRAT cleanup: make calculations and indenting level more sane

[keith <kmannth@us.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

On Thu, 2005-02-24 at 09:29, Dave Hansen wrote:
> Using the assumption that all addresses in the SRAT are ascending,
> the calculations can get a bit simpler, and remove the 
> "been_here_before" variable.
> 
> This also breaks that calculation out into its own function, which
> further simplifies the look of the code.
> 
> Signed-off-by: Dave Hansen <haveblue@us.ibm.com>
> ---
> 
>  sparse-dave/arch/i386/kernel/srat.c |   61 ++++++++++++++++++------------------
>  1 files changed, 32 insertions(+), 29 deletions(-)

(snip)

This looks a lot better then the existing code.  Thanks. 
 
Why not take it one step further??  Something like the attached patch.
There is no reason to loop over the nodes as the srat entries contain
node info and we can use the the new node_has_online_mem. 

This booted ok on my hot-add enabled 8-way. 
 I am not %100 sure it is ok to make the assumption that the memory is
always reported linearly but that is the assumption of the previous code
so it must be for all know examples. 

Keith 


[-- Attachment #2: patch-srat-cleanup-v2 --]
[-- Type: text/x-patch, Size: 2000 bytes --]

diff -urN linux-2.6.11-rc4-fix7/arch/i386/kernel/srat.c linux-2.6.11-rc4-fix7-srat/arch/i386/kernel/srat.c
--- linux-2.6.11-rc4-fix7/arch/i386/kernel/srat.c	2005-02-21 13:56:28.000000000 -0800
+++ linux-2.6.11-rc4-fix7-srat/arch/i386/kernel/srat.c	2005-02-24 11:10:09.000000000 -0800
@@ -269,34 +269,24 @@
 	}
  
 	/*calculate node_start_pfn/node_end_pfn arrays*/
-	for_each_online_node(nid) {
-		int been_here_before = 0;
-
-		for (j = 0; j < num_memory_chunks; j++){
-			/*
-			 *Only add present memroy to node_end/start_pfn 
-			 *There is no guarantee from the srat that the memory 
-			 *is present at boot time. 
-			 */
-			if (node_memory_chunk[j].start_pfn >= max_pfn) {
-				printk (KERN_INFO "Ignoring chunk of memory reported in the SRAT (could be hot-add zone?)\n");
-				printk (KERN_INFO "chunk is reported from pfn %04x to %04x\n",
-					node_memory_chunk[j].start_pfn, node_memory_chunk[j].end_pfn);
-				continue;
-			}
-			if (node_memory_chunk[j].nid == nid) {
-				if (been_here_before == 0) {
-					node_start_pfn[nid] = node_memory_chunk[j].start_pfn;
-					node_end_pfn[nid] = node_memory_chunk[j].end_pfn;
-					been_here_before = 1;
-				} else { /* We've found another chunk of memory for the node */
-					if (node_start_pfn[nid] < node_memory_chunk[j].start_pfn) {
-						node_end_pfn[nid] = node_memory_chunk[j].end_pfn;
-					}
-				}
-			}
+	for (j = 0; j < num_memory_chunks; j++){
+		int nid = node_memory_chunk[j].nid;
+		/*
+		 *Only add present memroy to node_end/start_pfn
+		 *There is no guarantee from the srat that the memory
+		 *is present at boot time.
+		 */
+		if (node_memory_chunk[j].start_pfn >= max_pfn) {
+			printk (KERN_INFO "Ignoring SRAT pfns: 0x%08lx -> %08lx\n",
+				node_memory_chunk[j].start_pfn, node_memory_chunk[j].end_pfn);
+			continue;
 		}
+		if (!node_has_online_mem(nid))
+			node_start_pfn[nid] = node_memory_chunk[j].start_pfn;
+			
+		node_end_pfn[nid] = node_memory_chunk[j].end_pfn;
 	}
+
 	return 1;
 out_fail:
 	return 0;