Re: [PATCH] x86: UV BAU activation descriptor init

From: Ingo Molnar
Date: Wed May 20 2009 - 04:34:21 EST



* Cliff Wickman <cpw@xxxxxxx> wrote:

> From: Cliff Wickman <cpw@xxxxxxx>
>
> The UV tlb shootdown code has a serious initialization error.
>
> An array of structures [32*8] is initialized as if it were [32].
> The array is indexed by (cpu number on the blade)*8, so the short
> initialization works for up to 4 cpus on a blade.
> But above that, we provide an invalid opcode to the hub's
> broadcast assist unit.
>
> This patch changes the allocation of the array to use its symbolic
> dimensions for better clarity. And initializes all 32*8 entries.
>
> Tested on the UV simulator.
>
> Diffed against 2.6.30-rc6
>
> Signed-off-by: Cliff Wickman <cpw@xxxxxxx>
>
> ---
> arch/x86/kernel/tlb_uv.c | 18 ++++++++++++++++--
> 1 file changed, 16 insertions(+), 2 deletions(-)
>
> Index: linux/arch/x86/kernel/tlb_uv.c
> ===================================================================
> --- linux.orig/arch/x86/kernel/tlb_uv.c
> +++ linux/arch/x86/kernel/tlb_uv.c
> @@ -715,7 +715,14 @@ uv_activation_descriptor_init(int node,
> struct bau_desc *adp;
> struct bau_desc *ad2;
>
> - adp = (struct bau_desc *)kmalloc_node(16384, GFP_KERNEL, node);
> + /*
> + * each bau_desc is 64 bytes; there are 8 (UV_ITEMS_PER_DESCRIPTOR)
> + * per cpu; and up to 32 (UV_ACTIVATION_DESCRIPTOR_SIZE) cpu's
> + * per blade
> + */
> + adp = (struct bau_desc *)kmalloc_node(sizeof(struct bau_desc)*
> + UV_ACTIVATION_DESCRIPTOR_SIZE*UV_ITEMS_PER_DESCRIPTOR,
> + GFP_KERNEL, node);
> BUG_ON(!adp);
>
> pa = uv_gpa(adp); /* need the real nasid*/
> @@ -729,7 +736,14 @@ uv_activation_descriptor_init(int node,
> (n << UV_DESC_BASE_PNODE_SHIFT | m));
> }
>
> - for (i = 0, ad2 = adp; i < UV_ACTIVATION_DESCRIPTOR_SIZE; i++, ad2++) {
> + /*
> + * initializing all 8 (UV_ITEMS_PER_DESCRIPTOR) descriptors for each
> + * cpu even though we only use the first one; one descriptor can
> + * describe a broadcast to 256 nodes.
> + */
> + for (i = 0, ad2 = adp;
> + i < (UV_ACTIVATION_DESCRIPTOR_SIZE*UV_ITEMS_PER_DESCRIPTOR);
> + i++, ad2++) {
> memset(ad2, 0, sizeof(struct bau_desc));
> ad2->header.sw_ack_flag = 1;
> /*

looks good but could you please define a shortcut for:

UV_ACTIVATION_DESCRIPTOR_SIZE*UV_ITEMS_PER_DESCRIPTOR

because its length causes a lot of ugly linebreaks. Something like
UV_ADP_SIZE should do the trick?

Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/