Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>


diff --git a/init/dyn_array.c b/init/dyn_array.c
index c4cd902..e85b1d6 100644
--- a/init/dyn_array.c
+++ b/init/dyn_array.c
@@ -17,10 +17,9 @@ void __init pre_alloc_dyn_array(void)
 	for (daa = __dyn_array_start ; daa < __dyn_array_end; daa++) {
 		struct dyn_array *da = *daa;
 
+		printk(KERN_DEBUG "dyn_array %pF size:%#lx nr:%d align:%#lx\n",
+			da->name, da->size, *da->nr, da->align);
 		size = da->size * (*da->nr);
-		print_fn_descriptor_symbol("dyn_array %s ", da->name);
-		printk(KERN_CONT "size:%#lx nr:%d align:%#lx\n",
-			da->size, *da->nr, da->align);
 		total_size += roundup(size, da->align);
 		if (da->align > max_align)
 			max_align = da->align;
@@ -42,11 +41,10 @@ void __init pre_alloc_dyn_array(void)
 		struct dyn_array *da = *daa;
 
 		size = da->size * (*da->nr);
-		print_fn_descriptor_symbol("dyn_array %s ", da->name);
-
 		phys = roundup(phys, da->align);
+		printk(KERN_DEBUG "dyn_array %pF ==> [%#lx - %#lx]\n",
+			da->name, phys, phys + size);
 		*da->name = phys_to_virt(phys);
-		printk(KERN_CONT " ==> [%#lx - %#lx]\n", phys, phys + size);
 
 		phys += size;
 
@@ -74,10 +72,9 @@ unsigned long __init per_cpu_dyn_array_size(unsigned long *align)
 	for (daa = __per_cpu_dyn_array_start ; daa < __per_cpu_dyn_array_end; daa++) {
 		struct dyn_array *da = *daa;
 
+		printk(KERN_DEBUG "per_cpu_dyn_array %pF size:%#lx nr:%d align:%#lx\n",
+			da->name, da->size, *da->nr, da->align);
 		size = da->size * (*da->nr);
-		print_fn_descriptor_symbol("per_cpu_dyn_array %s ", da->name);
-		printk(KERN_CONT "size:%#lx nr:%d align:%#lx\n",
-			da->size, *da->nr, da->align);
 		total_size += roundup(size, da->align);
 		if (da->align > max_align)
 			max_align = da->align;
@@ -104,15 +101,15 @@ void __init per_cpu_alloc_dyn_array(int cpu, char *ptr)
 		struct dyn_array *da = *daa;
 
 		size = da->size * (*da->nr);
-		print_fn_descriptor_symbol("per_cpu_dyn_array %s ", da->name);
-
 		phys = ...
[ message continues ]

On Fri, 29 Aug 2008 15:07:49 -0700

This:

struct dyn_array {
	void **name;

is a bit confusing.  One normally expects a variable called "name" to
point at a character string.

What _does_ this thing point at?  There are no code comments which I
can find, it's unobvious from the source code, the type is the
information-free void** and the identifier is misleading.

I find that documenting the data structures is the best way of making
code understandable (and hence maintainable).

--

[ message continues ]

On Fri, Aug 29, 2008 at 3:20 PM, Andrew Morton

struct dyn_array {
        void **name;
        unsigned long size;
        unsigned int *nr;
        unsigned long align;
        void (*init_work)(void *);
};
extern struct dyn_array *__dyn_array_start[], *__dyn_array_end[];
extern struct dyn_array *__per_cpu_dyn_array_start[],
*__per_cpu_dyn_array_end[];

#define DEFINE_DYN_ARRAY_ADDR(nameX, addrX, sizeX, nrX, alignX, init_workX) \
                static struct dyn_array __dyn_array_##nameX __initdata = \
                {       .name = (void **)&(nameX),\
                        .size = sizeX,\
                        .nr   = &(nrX),\
                        .align = alignX,\
                        .init_work = init_workX,\
                }; \
                static struct dyn_array *__dyn_array_ptr_##nameX __used \
                __attribute__((__section__(".dyn_array.init"))) = \
                        &__dyn_array_##nameX

#define DEFINE_DYN_ARRAY(nameX, sizeX, nrX, alignX, init_workX) \
        DEFINE_DYN_ARRAY_ADDR(nameX, nameX, sizeX, nrX, alignX, init_workX)

and use is

struct irq_desc *sparse_irqs;
DEFINE_DYN_ARRAY(sparse_irqs, sizeof(struct irq_desc), nr_irq_desc,
PAGE_SIZE, init_work);


then sparse_irqs is pointer, and .name store the address of that pointer.

later use
*da->name = phys_to_virt(phys);
to take back the dyn address.

YH
--

[ message continues ]

On Fri, 29 Aug 2008 15:32:58 -0700

Well yes, I have a copy of that too.

Why is it called "name"?

--

[ message continues ]

That doesn't appear to have been very well tested?

The code has a few coding-style glitches which checkpatch can detect.
--

[ message continues ]

On Fri, Aug 29, 2008 at 3:53 PM, Andrew Morton


ah!

should only have 80 char length warning...

YH
--

[ message continues ]

On Fri, 29 Aug 2008 16:34:26 -0700


sure.  The code looks rather miserable in an 80-col display.

there's also

WARNING: braces {} are not necessary for single statement blocks
#119: FILE: dyn_array.c:119:
+               if (da->init_work) {
+                       da->init_work(da);
+               }



and checkpatch should have detected the misplaced semicolon here:

+       for (daa = __per_cpu_dyn_array_start ; daa < __per_cpu_dyn_array_end; daa++) {

but didn't.

		
--

[ message continues ]

on non-genirq systems? Most likely. If then most testing they get is 

yeah. Yinghai, could you please fix them?

	Ingo
--

[ message continues ]

already in tip/master and -mm
except  first one.

YH
--

[ message continues ]

that needs fixing too i think. We dont really want to sprinkle the code 
with various specific panics.

	Ingo
--

[ message continues ]

ok, please check that in another mail.

YH
--

[ message continues ]