Re: [PATCH] more discontig fun

On Tue, Jul 29, 2003 at 03:43:58PM -0700, David Mosberger wrote:
> Hi Jesse,
> 
> Two quick comments:
> 
> +
> +	/* for discontig machines, we do this in discontig.c */
>  	if (smp_processor_id() == 0) {
>  		cpu_data = __alloc_bootmem(PERCPU_PAGE_SIZE * NR_CPUS, PERCPU_PAGE_SIZE,
> 
> This comment seems out of place.  Isn't it trying to say that all of
> per_cpu_init() is done in discontig.c for NUMA?  If so the comment
> should be in front of the #ifdef around CONFIG_DISCONTIGMEM.  Also,
> I'm wondering whether we shouldn't split off the contig-mem case into
> a separate file.  I don't like those #ifdef CONFIG_DISCONTIGMEM.  It
> would be better to control that via the Makefile.

A quick grep shows quite a few #ifndef CONFIG_DISCONTIGMEMs, I'll see if
I can clean some stuff up and put it into contig.c.

> On to this one:
> 
> +struct memmap_count_callback_data {
> +	int node;
> +	unsigned long num_physpages;
> +	unsigned long num_dma_physpages;
> +	unsigned long min_pfn;
> +	unsigned long max_pfn;
> +} cdata;
> 
> I don't like non-reentrant code.  Can't you allocate the variable on
> the stack and pass a pointer to it via the callback interface?  Yes,
> we may not be needing re-entrancy right now, but I just think it's bad
> design to have non-reentrant code (unless there are good reasons for
> it).

Ok, I'll fix this too.  Thanks for looking at it.

Jesse
-
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html