On Thu, 2010-01-14 at 14:41 -0600, Joel Schopp wrote:
> In the previous patch the client-architecture field for the number of
> cores supported is set statically as high as is possible. However, that
> static setting could be too high if the system supports smt, resulting
> in cpus assigned to Linux that are not booted. This patch reads the
> device tree (before it is unflattened) to determine the amount of smt.
> It then dynamically updates the entires in the array with the proper
> number of cores supported. Tests show this correctly detecting SMT4 on
> a Power7 and still booting all the supported cores on a large machine.
Same comments about submission format as the previous patch...
> Signed-off-by:Joel Schopp<jsch...@austin.ibm.com>
> Index: linux-2.6.git/arch/powerpc/kernel/prom_init.c
> ===================================================================
> --- linux-2.6.git.orig/arch/powerpc/kernel/prom_init.c
> +++ linux-2.6.git/arch/powerpc/kernel/prom_init.c
> @@ -141,6 +141,8 @@ typedef u32 cell_t;
>
> extern void __start(unsigned long r3, unsigned long r4, unsigned long r5);
>
> +static int __init prom_smt_way(void);
Do you really need a forward declaration ? We generally avoid those and
prefer instead having the functions in the right order to make it
unnecessary.
> #ifdef CONFIG_PPC64
> extern int enter_prom(struct prom_args *args, unsigned long entry);
> #else
> @@ -811,9 +813,17 @@ static void __init prom_send_capabilitie
> {
> ihandle elfloader, root;
> prom_arg_t ret;
> + u32 *cores;
>
> root = call_prom("open", 1, 1, ADDR("/"));
> if (root != 0) {
> + /*
> + * If you add to the struct, please be sure the 100 index
> + * didn't change. The BUILD_BUG_ON is a reminder.
> + */
> + BUILD_BUG_ON(sizeof(ibm_architecture_vec) != 108);
This is indeed a bit fishy... a nicer way may have been to have the
vector in an asm file with labels but that's probably overkill. Just
maybe add a runtime test that checks the value read initially is
NR_CORES and if not, print a big fat warning ?
> + cores = (u32 *) &ibm_architecture_vec[100];
> + *cores = (u32) (NR_CPUS/prom_smt_way());
The style is a bit gross though I suppose it will do. Drop the cast on
the second line, it's not useful, and stick some spaces around that
division. Also maybe print out a message saying to what value you
adjusted the max number of supported cores, might be useful on the field
to diagnose issues.
> /* try calling the ibm,client-architecture-support method */
> prom_printf("Calling ibm,client-architecture-support...");
> if (call_prom_ret("call-method", 3, 2, &ret,
> @@ -1031,6 +1041,45 @@ static void __init reserve_mem(u64 base,
> RELOC(mem_reserve_cnt) = cnt + 1;
> }
>
> +
> +static int __init prom_smt_way(void)
> +{
> + phandle node;
> + char type[64];
> + unsigned int plen;
> +
> + for (node = 0; prom_next_node(&node); ) {
> + type[0] = 0;
> + prom_getprop(node, "device_type", type, sizeof(type));
> +
> + if (type[0] == 0) {
> + /*
> + * CHRP Longtrail machines have no device_type
> + * on the memory node, so check the name instead...
> + */
> + prom_getprop(node, "name", type, sizeof(type));
They also have no SMT :-) Just continue instead. You could also have
instead gone for /cpus and peeked at the first child. Might have lead to
a simpler construct (and faster runtime)
> + }
> + if (strcmp(type, RELOC("cpu")))
> + continue;
> +
> + /*
> + * There is an entry for each smt thread, each entry being
> + * 4 bytes long. All cpus should have the same number of
> + * smt threads, so return after finding the first.
> + */
> + plen = prom_getproplen(node, "ibm,ppc-interrupt-server#s");
> + prom_debug("smt %x\n", (unsigned long) plen);
Might only be a debug message but it should be a tad more verbose about
what it's actually printing.
> + if (plen >= 4)
> + return plen / 4;
> + }
> + /*
> + * If things go wrong and we get here fallback to SMT1
> + */
> + prom_debug("unable to determine smt from device tree, guessing smt1\n");
> + return 1;
> +
> +}
> +
Cheers,
Ben.
> /*
> * Initialize memory allocation mechanism, parse "memory" nodes and
> * obtain that way the top of memory and RMO to setup out local allocator
>
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
