Re: [patch 08/16] Add support for X86_64 platforms to KGDB

[Andi Kleen <ak@suse.de>]

On Monday 29 August 2005 18:10, Tom Rini wrote:

> +void __init early_setup_per_cpu_area(void)
> +{
> +	static char cpu0[PERCPU_ENOUGH_ROOM]
> +		__attribute__ ((aligned (SMP_CACHE_BYTES)));
> +	char *ptr = cpu0;
> +
> +	cpu_pda[0].data_offset = ptr - __per_cpu_start;
> +	memcpy(ptr, __per_cpu_start, __per_cpu_end - __per_cpu_start);
> +}

What is that?  It looks totally bogus. Can you tell exactly where you
believe early per cpu data is needed?

> +
>  /*
>   * Great future plan:
>   * Declare PDA itself and support (irqstack,tss,pgd) as per cpu data.
> @@ -97,7 +107,9 @@ void __init setup_per_cpu_areas(void)
>  	for (i = 0; i < NR_CPUS; i++) {
>  		char *ptr;
>
> -		if (!NODE_DATA(cpu_to_node(i))) {
> +		if (cpu_pda[i].data_offset)
> +			continue;

And that looks broken too.

In general I would also advise to mix any other changes outside kgdb* into the 
x86-64 kgdb patch. Either the patch should be merged into mainline in a 
separate patch or kgdb reworked to not need this.

> +	if (notify_die(DIE_PAGE_FAULT, "no context", regs, error_code, 14,
> +				SIGSEGV) == NOTIFY_STOP)
> +		return;
> +

I can see the point of that. It's ok if you submit it as a separate patch.

Regarding early trap init: I would have no problem to move all of traps_init
into setup_arch (and leave traps_init empty for generic code). I actually
don't know why it runs so late. But doing it half way is ugly.

-Andi