2005-09-30 18:55:25

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 07/07] i386: numa emulation on pc

On Fri, 2005-09-30 at 16:33 +0900, Magnus Damm wrote:
> void __init nid_zone_sizes_init(int nid)
> {
> unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
> - unsigned long max_dma;
> + unsigned long max_dma = min(max_hardware_dma_pfn(), max_low_pfn);
> unsigned long start = node_start_pfn[nid];
> unsigned long end = node_end_pfn[nid];
>
> if (node_has_online_mem(nid)){
> - if (nid_starts_in_highmem(nid)) {
> - zones_size[ZONE_HIGHMEM] = nid_size_pages(nid);
> - } else {
> - max_dma = min(max_hardware_dma_pfn(), max_low_pfn);
> - zones_size[ZONE_DMA] = max_dma;
> - zones_size[ZONE_NORMAL] = max_low_pfn - max_dma;
> - zones_size[ZONE_HIGHMEM] = end - max_low_pfn;
> + if (start < max_dma) {
> + zones_size[ZONE_DMA] = min(end, max_dma) - start;
> + }
> + if (start < max_low_pfn && max_dma < end) {
> + zones_size[ZONE_NORMAL] = min(end, max_low_pfn) - max(start, max_dma);
> + }
> + if (max_low_pfn <= end) {
> + zones_size[ZONE_HIGHMEM] = end - max(start, max_low_pfn);
> }
> }

That is a decent cleanup all by itself. You might want to break it out.
Take a look at the patches I just sent out. They do some similar things
to the same code.

> @@ -1270,7 +1273,12 @@ void __init setup_bootmem_allocator(void
> /*
> * Initialize the boot-time allocator (with low memory only):
> */
> +#ifdef CONFIG_NUMA_EMU
> + bootmap_size = init_bootmem(max(min_low_pfn, node_start_pfn[0]),
> + min(max_low_pfn, node_end_pfn[0]));
> +#else
> bootmap_size = init_bootmem(min_low_pfn, max_low_pfn);
> +#endif

This shouldn't be necessary. Again, take a look at my discontig
separation patches and see if what I did works for you here.

> register_bootmem_low_pages(max_low_pfn);
>
> --- from-0006/arch/i386/mm/numa.c
> +++ to-work/arch/i386/mm/numa.c 2005-09-28 17:49:53.000000000 +0900
> @@ -165,3 +165,103 @@ int early_pfn_to_nid(unsigned long pfn)
>
> return 0;
> }
> +
> +#ifdef CONFIG_NUMA_EMU
...
> +#endif

Ewwwwww :) No real need to put new function in a big #ifdef like that.
Can you just create a new file for NUMA emulation?

> --- from-0001/include/asm-i386/numnodes.h
> +++ to-work/include/asm-i386/numnodes.h 2005-09-28 17:49:53.000000000 +0900
> @@ -8,7 +8,7 @@
> /* Max 16 Nodes */
> #define NODES_SHIFT 4
>
> -#elif defined(CONFIG_ACPI_SRAT)
> +#elif defined(CONFIG_ACPI_SRAT) || defined(CONFIG_NUMA_EMU)
>
> /* Max 8 Nodes */
> #define NODES_SHIFT 3

Geez. We should probably just do those in the Kconfig files. Would
look much simpler. But, that's a patch for another day. This is fine
by itself.

-- Dave


2005-10-03 09:59:42

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 07/07] i386: numa emulation on pc

Hi again Dave,

On 10/1/05, Dave Hansen <[email protected]> wrote:
> On Fri, 2005-09-30 at 16:33 +0900, Magnus Damm wrote:
> > void __init nid_zone_sizes_init(int nid)
> > {
> > unsigned long zones_size[MAX_NR_ZONES] = {0, 0, 0};
> > - unsigned long max_dma;
> > + unsigned long max_dma = min(max_hardware_dma_pfn(), max_low_pfn);
> > unsigned long start = node_start_pfn[nid];
> > unsigned long end = node_end_pfn[nid];
> >
> > if (node_has_online_mem(nid)){
> > - if (nid_starts_in_highmem(nid)) {
> > - zones_size[ZONE_HIGHMEM] = nid_size_pages(nid);
> > - } else {
> > - max_dma = min(max_hardware_dma_pfn(), max_low_pfn);
> > - zones_size[ZONE_DMA] = max_dma;
> > - zones_size[ZONE_NORMAL] = max_low_pfn - max_dma;
> > - zones_size[ZONE_HIGHMEM] = end - max_low_pfn;
> > + if (start < max_dma) {
> > + zones_size[ZONE_DMA] = min(end, max_dma) - start;
> > + }
> > + if (start < max_low_pfn && max_dma < end) {
> > + zones_size[ZONE_NORMAL] = min(end, max_low_pfn) - max(start, max_dma);
> > + }
> > + if (max_low_pfn <= end) {
> > + zones_size[ZONE_HIGHMEM] = end - max(start, max_low_pfn);
> > }
> > }
>
> That is a decent cleanup all by itself. You might want to break it out.
> Take a look at the patches I just sent out. They do some similar things
> to the same code.

Break it out, sure! I'm not sure which patch to look at, though.

> > @@ -1270,7 +1273,12 @@ void __init setup_bootmem_allocator(void
> > /*
> > * Initialize the boot-time allocator (with low memory only):
> > */
> > +#ifdef CONFIG_NUMA_EMU
> > + bootmap_size = init_bootmem(max(min_low_pfn, node_start_pfn[0]),
> > + min(max_low_pfn, node_end_pfn[0]));
> > +#else
> > bootmap_size = init_bootmem(min_low_pfn, max_low_pfn);
> > +#endif
>
> This shouldn't be necessary. Again, take a look at my discontig
> separation patches and see if what I did works for you here.

Do you mean "discontig-consolidate0.patch"? Maybe I'm misunderstanding.

> > +#ifdef CONFIG_NUMA_EMU
> ...
> > +#endif
>
> Ewwwwww :) No real need to put new function in a big #ifdef like that.
> Can you just create a new file for NUMA emulation?

Hehe, what is this, a beauty contest? =) I agree, but I guess the
reason for this code to be here is that a similar arrangement is done
by x86_64...

I will create a new file. Is arch/i386/mm/numa_emu.c good?

> > --- from-0001/include/asm-i386/numnodes.h
> > +++ to-work/include/asm-i386/numnodes.h 2005-09-28 17:49:53.000000000 +0900
> > @@ -8,7 +8,7 @@
> > /* Max 16 Nodes */
> > #define NODES_SHIFT 4
> >
> > -#elif defined(CONFIG_ACPI_SRAT)
> > +#elif defined(CONFIG_ACPI_SRAT) || defined(CONFIG_NUMA_EMU)
> >
> > /* Max 8 Nodes */
> > #define NODES_SHIFT 3
>
> Geez. We should probably just do those in the Kconfig files. Would
> look much simpler. But, that's a patch for another day. This is fine
> by itself.

No biggie, I will add a config option.

But first, you have written lots and lots of patches, and I am
confused. Could you please tell me on which patches I should base my
code to make things as easy as possible?

Many thanks,

/ magnus

2005-10-03 16:16:46

by Dave Hansen

[permalink] [raw]
Subject: Re: [PATCH 07/07] i386: numa emulation on pc

On Mon, 2005-10-03 at 18:59 +0900, Magnus Damm wrote:
> > > +#ifdef CONFIG_NUMA_EMU
> > > + bootmap_size = init_bootmem(max(min_low_pfn, node_start_pfn[0]),
> > > + min(max_low_pfn, node_end_pfn[0]));
> > > +#else
> > > bootmap_size = init_bootmem(min_low_pfn, max_low_pfn);
> > > +#endif
> >
> > This shouldn't be necessary. Again, take a look at my discontig
> > separation patches and see if what I did works for you here.
>
> Do you mean "discontig-consolidate0.patch"? Maybe I'm misunderstanding.

This one, I believe:

http://sr71.net/patches/2.6.14/2.6.14-rc2-git8-mhp1/broken-out/B2.1-i386-discontig-consolidation.patch

> > > +#ifdef CONFIG_NUMA_EMU
> > ...
> > > +#endif
> >
> > Ewwwwww :) No real need to put new function in a big #ifdef like that.
> > Can you just create a new file for NUMA emulation?
>
> Hehe, what is this, a beauty contest? =) I agree, but I guess the
> reason for this code to be here is that a similar arrangement is done
> by x86_64...

If that's really the case, can they _actually_ share code? Maybe we can
do this NUMA emulation thing in non-arch code. Just guessing...

> I will create a new file. Is arch/i386/mm/numa_emu.c good?

> But first, you have written lots and lots of patches, and I am
> confused. Could you please tell me on which patches I should base my
> code to make things as easy as possible?

This is the staging ground for my memory hotplug work. But, it contains
all of my work on other stuff, too. If you build on top of this, it
would be great:

http://sr71.net/patches/2.6.14/2.6.14-rc2-git8-mhp1/

-- Dave

2005-10-04 05:06:57

by Magnus Damm

[permalink] [raw]
Subject: Re: [PATCH 07/07] i386: numa emulation on pc

On 10/4/05, Dave Hansen <[email protected]> wrote:
> On Mon, 2005-10-03 at 18:59 +0900, Magnus Damm wrote:
> > > > +#ifdef CONFIG_NUMA_EMU
> > > ...
> > > > +#endif
> > >
> > > Ewwwwww :) No real need to put new function in a big #ifdef like that.
> > > Can you just create a new file for NUMA emulation?
> >
> > Hehe, what is this, a beauty contest? =) I agree, but I guess the
> > reason for this code to be here is that a similar arrangement is done
> > by x86_64...
>
> If that's really the case, can they _actually_ share code? Maybe we can
> do this NUMA emulation thing in non-arch code. Just guessing...

I'd like to avoid duplication as much as you, but at a quick glance
the x86_64 and i386 architecture looked pretty different. But I will
see what I can do.

> > I will create a new file. Is arch/i386/mm/numa_emu.c good?
>
> > But first, you have written lots and lots of patches, and I am
> > confused. Could you please tell me on which patches I should base my
> > code to make things as easy as possible?
>
> This is the staging ground for my memory hotplug work. But, it contains
> all of my work on other stuff, too. If you build on top of this, it
> would be great:
>
> http://sr71.net/patches/2.6.14/2.6.14-rc2-git8-mhp1/

I will build on top of that then.

Thanks,

/ magnus