No subject
Michael Neuling
mikey at neuling.org
Mon Jul 7 21:56:25 EDT 2008
> [PATCH 1/4] kdump : add support for ibm, dynamic-reconfiguration-memory for kexec/kdump
Please change the name of the individual patches
BTW this patch doesn't apply against the powerpc-next tree. Please use
that tree, not Linus tree.
In message <200807080014.24910.chandru at in.ibm.com> you wrote:
> kexec-tools adds crash, rtas, and tce memory regions as linux,usable-memory
> properties in device-tree. Following changes are made in the kernel to
> recognize these special properties in case of
> ibm,dynamic-reconfiguration-memory node of device-tree.
>
> Signed-off-by: Chandru Siddalingappa <chandru at in.ibm.com>
> ---
>
> diff -Naurp linux-2.6.26-rc9-orig/arch/powerpc/kernel/prom.c
> linux-2.6.26-rc9/arch/powerpc/kernel/prom.c
> --- linux-2.6.26-rc9-orig/arch/powerpc/kernel/prom.c 2008-07-06
> 04:23:22.000000000 +0530
> +++ linux-2.6.26-rc9/arch/powerpc/kernel/prom.c 2008-07-07 17:23:58.000
000000
> +0530
> @@ -884,9 +884,10 @@ static u64 __init dt_mem_next_cell(int s
> */
> static int __init early_init_dt_scan_drconf_memory(unsigned long node)
> {
> - cell_t *dm, *ls;
> + cell_t *dm, *ls, *endp, *usm;
> unsigned long l, n, flags;
> u64 base, size, lmb_size;
> + char buf[32], t[8];
>
> ls = (cell_t *)of_get_flat_dt_prop(node, "ibm,lmb-size", &l);
> if (ls == NULL || l < dt_root_size_cells * sizeof(cell_t))
> @@ -917,7 +918,33 @@ static int __init early_init_dt_scan_drc
> if ((base + size) > 0x80000000ul)
> size = 0x80000000ul - base;
> }
Please document what you are trying to achieve here
> - lmb_add(base, size);
> + strcpy(buf, "linux,usable-memory");
> + sprintf(t, "%d", (int)n);
> + strcat(buf, t);
sprintf(buf, "linux,usable-memory%d", (int)n);
??
> + usm = (cell_t *)of_get_flat_dt_prop(node,
> + (const char *)buf, &l);
> + if (usm != NULL) {
> + endp = usm + (l / sizeof(cell_t));
> + while ((endp - usm) >= (dt_root_addr_cells +
> + dt_root_size_cells)) {
> + base = dt_mem_next_cell(dt_root_addr_cells,
> + &usm);
> + size = dt_mem_next_cell(dt_root_size_cells,
> + &usm);
> + if (size == 0)
> + continue;
> + if (iommu_is_off) {
> + if ((base + size) > 0x80000000ul)
> + size = 0x80000000ul - base;
There is similar code to this above. Can this be merged? Why
0x80000000ul anyway?
> + }
> + lmb_add(base, size);
> + }
> +
> + /* Continue with next lmb entry */
> + continue;
Is this "continue" needed?
Also, this comment is useless. We know what a continue does :-)
> + } else {
> + lmb_add(base, size);
> + }
> }
> lmb_dump_all();
> return 0;
> diff -Naurp linux-2.6.26-rc9-orig/arch/powerpc/mm/numa.c
> linux-2.6.26-rc9/arch/powerpc/mm/numa.c
> --- linux-2.6.26-rc9-orig/arch/powerpc/mm/numa.c 2008-07-06 04:23:22.000
000000
> +0530
> +++ linux-2.6.26-rc9/arch/powerpc/mm/numa.c 2008-07-07 17:50:35.000000000
> +0530
> @@ -349,18 +349,33 @@ static unsigned long __init numa_enforce
> return lmb_end_of_DRAM() - start;
> }
>
> +static void set_nodeinfo(int nid, unsigned long start, unsigned long size)
> +{
> + fake_numa_create_new_node(((start + size) >> PAGE_SHIFT),
> + &nid);
> + node_set_online(nid);
> +
> + size = numa_enforce_memory_limit(start, size);
> + if (!size)
> + return;
> + add_active_range(nid, start >> PAGE_SHIFT,
> + (start >> PAGE_SHIFT) + (size >> PAGE_SHIFT));
> + return;
> +}
> +
> /*
> * Extract NUMA information from the ibm,dynamic-reconfiguration-memory
> * node. This assumes n_mem_{addr,size}_cells have been set.
> */
> static void __init parse_drconf_memory(struct device_node *memory)
> {
> - const unsigned int *lm, *dm, *aa;
> + const unsigned int *lm, *dm, *aa, *usm;
> unsigned int ls, ld, la;
> unsigned int n, aam, aalen;
> unsigned long lmb_size, size, start;
> int nid, default_nid = 0;
> - unsigned int ai, flags;
> + unsigned int ai, flags, len, ranges;
> + char buf[32], t[8];
>
> lm = of_get_property(memory, "ibm,lmb-size", &ls);
> dm = of_get_property(memory, "ibm,dynamic-memory", &ld);
> @@ -396,16 +411,27 @@ static void __init parse_drconf_memory(s
> nid = default_nid;
> }
>
> - fake_numa_create_new_node(((start + lmb_size) >> PAGE_SHIFT),
> - &nid);
> - node_set_online(nid);
> + strcpy(buf, "linux,usable-memory");
> + sprintf(t, "%d", (int)n);
> + strcat(buf, t);
Again, we can do this in a single sprintf, like above...
> + usm = of_get_property(memory, (const char *)buf, &len);
> + if (usm != NULL) {
> + ranges = (len >> 2) / (n_mem_addr_cells +
> + n_mem_size_cells);
> +
> +dr_new_range: start = read_n_cells(n_mem_addr_cells, &usm);
> + size = read_n_cells(n_mem_size_cells, &usm);
> + if (size == 0)
> + continue;
>
> - size = numa_enforce_memory_limit(start, lmb_size);
> - if (!size)
> - continue;
> + set_nodeinfo(nid, start, size);
> + if (--ranges)
> + goto dr_new_range;
Can you please use a while or for loop rather than a goto.
>
> - add_active_range(nid, start >> PAGE_SHIFT,
> - (start >> PAGE_SHIFT) + (size >> PAGE_SHIFT));
> + continue;
> + } else {
> + set_nodeinfo(nid, start, lmb_size);
> + }
> }
> }
>
>
> _______________________________________________
> kexec mailing list
> kexec at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
>
More information about the kexec
mailing list