Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

[Alexey Dobriyan]

> --- 25/arch/i386/mm/discontig.c~x86-abstract-discontigmem-setup
> +++ 25-akpm/arch/i386/mm/discontig.c

> +void memory_present(int nid, unsigned long start, unsigned long end)
> +{
> +	unsigned long pfn;
> +
> +	printk(KERN_INFO "Node: %d, start_pfn: %ld, end_pfn: %ld\n",
> +			nid, start, end);
> +	printk(KERN_DEBUG "  Setting physnode_map array to node %d for pfns:\n", nid);
> +	printk(KERN_DEBUG "  ");
> +	for (pfn = start; pfn < end; pfn += PAGES_PER_ELEMENT) {
> +		physnode_map[pfn / PAGES_PER_ELEMENT] = nid;
> +		printk(KERN_DEBUG "%ld ", pfn);
		       ^^^^^^^^^^

> +	}
> +	printk(KERN_DEBUG "\n");
	       ^^^^^^^^^^
> +}

Too much KERN_DEBUG.

> --- 25/include/linux/mmzone.h~x86-abstract-discontigmem-setup
> +++ 25-akpm/include/linux/mmzone.h

> +#ifdef CONFIG_HAVE_MEMORY_PRESENT
> +void memory_present(int nid, unsigned long start, unsigned long end);
> +#else
> +static inline void memory_present(int nid, unsigned long start, unsigned long end) {}
> +#endif

> +#ifdef CONFIG_NEED_NODE_MEMMAP_SIZE
> +unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
> +#endif

	#else
	static inline unsigned long node_memmap_size_bytes(...);
	#endif

Is this needed?

	Alexey

Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

[Dave Hansen]

On Sat, 2005-03-05 at 15:35 +0200, Alexey Dobriyan wrote:
> > +	}
> > +	printk(KERN_DEBUG "\n");
> 	       ^^^^^^^^^^
> > +}
> 
> Too much KERN_DEBUG.

On my system, that ends up printing out 4 or 5 lines of output per node,
but it's quite invaluable if you're debugging early memory setup issues.
It is KERN_DEBUG after all.  What does it do on your system?

I'm not horribly opposed to removing some of this output, let's just
make sure...

> > --- 25/include/linux/mmzone.h~x86-abstract-discontigmem-setup
> > +++ 25-akpm/include/linux/mmzone.h
> 
> > +#ifdef CONFIG_HAVE_MEMORY_PRESENT
> > +void memory_present(int nid, unsigned long start, unsigned long end);
> > +#else
> > +static inline void memory_present(int nid, unsigned long start, unsigned long end) {}
> > +#endif
> 
> > +#ifdef CONFIG_NEED_NODE_MEMMAP_SIZE
> > +unsigned long __init node_memmap_size_bytes(int, unsigned long, unsigned long);
> > +#endif
> 
> 	#else
> 	static inline unsigned long node_memmap_size_bytes(...);
> 	#endif
> 
> Is this needed?

It's really only used for the i386 NUMA architectures, but it is
necessary.  We'll be overriding that discontigmem version for sparsemem,
which I'll be submitting soon:

http://www.sr71.net/patches/2.6.11/2.6.11-mhp1/broken-out/B-sparse-150-sparsemem.patch


-- Dave

Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

[Alexey Dobriyan]

On Saturday 05 March 2005 20:58, Dave Hansen wrote:
> On Sat, 2005-03-05 at 15:35 +0200, Alexey Dobriyan wrote:
> > > +	}
> > > +	printk(KERN_DEBUG "\n");
> > 	       ^^^^^^^^^^
> > > +}
> > 
> > Too much KERN_DEBUG.
> 
> On my system, that ends up printing out 4 or 5 lines of output per node,
> but it's quite invaluable if you're debugging early memory setup issues.
> It is KERN_DEBUG after all.  What does it do on your system?
> 
> I'm not horribly opposed to removing some of this output, let's just
> make sure...

You misundestood. I'm not proposing to remove these printk's altogether. I'm
for removing KERN_DEBUG solely in the middle of the line.

Try the following program with and without 3-rd and 4-th KERN_DEBUG.

	Alexey
============================================================================
#include <stdio.h>

#define KERN_DEBUG "<7>"

int main(void)
{
        int i;

        printf(KERN_DEBUG "  Setting physnode_map array to node:\n");
        printf(KERN_DEBUG "  ");
        for (i = 0; i < 10; i++)
                printf(KERN_DEBUG "%d ", i);
        printf(KERN_DEBUG "\n");

        return 0;
}

Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

[Dave Hansen]

On Sun, 2005-03-06 at 01:21 +0200, Alexey Dobriyan wrote:
> On Saturday 05 March 2005 20:58, Dave Hansen wrote:
> > On Sat, 2005-03-05 at 15:35 +0200, Alexey Dobriyan wrote:
> > > > +	}
> > > > +	printk(KERN_DEBUG "\n");
> > > 	       ^^^^^^^^^^
> > > > +}
> > > 
> > > Too much KERN_DEBUG.
> > 
> > On my system, that ends up printing out 4 or 5 lines of output per node,
> > but it's quite invaluable if you're debugging early memory setup issues.
> > It is KERN_DEBUG after all.  What does it do on your system?
> > 
> > I'm not horribly opposed to removing some of this output, let's just
> > make sure...
> 
> You misundestood. I'm not proposing to remove these printk's altogether. I'm
> for removing KERN_DEBUG solely in the middle of the line.
> 
> Try the following program with and without 3-rd and 4-th KERN_DEBUG.

Yep, I misunderstood :)



-- Dave

Re: 2.6.11-mm1 (x86-abstract-discontigmem-setup.patch)

[Dave Hansen]

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

I hit send on that other one accidentally...

I believe the attached patch should fix the extra output.  Compiles on
my normal summit discontig configuration.  Andrew, please apply some
time after x86-abstract-discontigmem-setup-fix.patch in your series.

-- Dave

[-- Attachment #2: x86-abstract-discontigmem-setup-fix-printk.patch --]
[-- Type: text/x-patch, Size: 778 bytes --]



---

 clean-dave/arch/i386/mm/discontig.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -puN arch/i386/mm/discontig.c~x86-abstract-discontigmem-setup-fix-printk arch/i386/mm/discontig.c
--- clean/arch/i386/mm/discontig.c~x86-abstract-discontigmem-setup-fix-printk	2005-03-07 10:40:53.000000000 -0800
+++ clean-dave/arch/i386/mm/discontig.c	2005-03-07 10:41:29.000000000 -0800
@@ -70,9 +70,9 @@ void memory_present(int nid, unsigned lo
 	printk(KERN_DEBUG "  ");
 	for (pfn = start; pfn < end; pfn += PAGES_PER_ELEMENT) {
 		physnode_map[pfn / PAGES_PER_ELEMENT] = nid;
-		printk(KERN_DEBUG "%ld ", pfn);
+		printk("%ld ", pfn);
 	}
-	printk(KERN_DEBUG "\n");
+	printk("\n");
 }
 
 unsigned long node_memmap_size_bytes(int nid, unsigned long start_pfn,
_