cpumask move patch revised - RFC

Joel Schopp jschopp at austin.ibm.com
Fri Aug 13 05:05:03 EST 2004


This will surely conflict with Nathan's recent patch "[patch 4/4] Remove
unnecessary cpu maps (available, present_at_boot)".  I think Nathan's
patch should go in first and yours reworked to match.  Other comments
inline below.

R Sharada wrote:

> Hello,
> 	Based on the feedback, here is the revised cpumask patch that
> moves the cpumask initialization from prom_hold_cpus() to later boot, in
> setup_system().
> The patch is against the 2.6.8-rc2 linus bitkeeper tree.
>
> - The get_property call has been corrected to obtain the property size from
> the correct argument.
> - The unnecessary variable initializations have been removed
> - check for NULL value of status incorporated.
>
> I have not removed the #ifdefs for SMP, as all the cpumask data structures,
> as I see them in code now, are defined for SMP systems and does not seem to be
> defined for UP.
> The merge of the POWERMAC and PSERIES #ifdefs is also deferred as I don't
> know a lot about the POWERMAC initialization and startup to see if the two
> cases can be merged.
>
> Please review and comment on the patch.
>
> Thanks and Regards,
> Sharada
>
>
> ------------------------------------------------------------------------
>
> diff -Naur linux-2.6.8-rc2-org/arch/ppc64/kernel/chrp_setup.c linux-2.6.8-rc2-chg/arch/ppc64/kernel/chrp_setup.c
> --- linux-2.6.8-rc2-org/arch/ppc64/kernel/chrp_setup.c	2004-08-03 02:12:58.000000000 -0700
> +++ linux-2.6.8-rc2-chg/arch/ppc64/kernel/chrp_setup.c	2004-08-13 06:02:22.808964544 -0700
> @@ -77,6 +77,8 @@
>  void pSeries_calibrate_decr(void);
>  void fwnmi_init(void);
>  extern void SystemReset_FWNMI(void), MachineCheck_FWNMI(void);	/* from head.S */
> +void cpumask_setup(void);
> +

Is this really necessary?  Might it go better in a .h file somewhere?

>  int fwnmi_active;  /* TRUE if an FWNMI handler is present */
>
>  dev_t boot_dev;
> @@ -468,3 +470,92 @@
>
>  	setup_default_decr();
>  }
> +
> +void cpumask_setup()
> +{
> +	unsigned long ind;
> +	struct device_node *np = NULL;
> +	int cpuid = 0;
> +	unsigned int *reg;
> +	char *statusp;
> +	int prop;
> +	int *propsize = ∝
> +	unsigned int cpu_threads;
> +
> +	printk(KERN_INFO "cpumask_setup\n");
> +	/* On pmac, we just fill out the various global bitmasks and
> +	* arrays indicating our CPUs are here, they are actually started
> +	* later on from pmac_smp
> +	*/
> +	if (systemcfg->platform == PLATFORM_POWERMAC) {
> +		while ((np = of_find_node_by_type(np, "cpu"))) {
> +			reg = (unsigned int *)get_property(np, "reg", NULL);
> +#ifdef CONFIG_SMP
> +			cpu_set(cpuid, cpu_available_map);
> +			cpu_set(cpuid, cpu_possible_map);
> +			cpu_set(cpuid, cpu_present_at_boot);
> +			if (*reg == 0)
> +				cpu_set(cpuid, cpu_online_map);
> +#endif /* CONFIG_SMP */
> +			cpuid++;
> +		}

Shouldn't the whole while loop and of_node_put be in the #ifdef
CONFIG_SMP, as otherwise all we do is iterate over the cpus not doing
anything?

> +		of_node_put(np);
> +		return;
> +	}
> +
> +	while ((np = of_find_node_by_type(np, "cpu"))) {
> +
> +		statusp = (char *)get_property(np, "status", NULL);
> +		if ((statusp == NULL) || (statusp && strcmp(statusp, "okay") != 0))
> +			continue;
> +
> +		reg = (unsigned int *)get_property(np, "reg", NULL);
> +
> +		get_property(np, "ibm,ppc-interrupt-server#s", propsize);
> +		if (*propsize < 0) {
> +			/* no property.  old hardware has no SMT */
> +			cpu_threads = 1;
> +		} else {
> +			/* We have a threaded processor */
> +			cpu_threads = *propsize / sizeof(u32);
> +			if (cpu_threads > 2)
> +			cpu_threads = 1; /* ToDo: panic? */

I think it is about time we start making code that will deal with more
than 2 cpu_threads, as the processors seem inevitable and not too far off.

> +		}
> +
> +#ifdef CONFIG_SMP
> +		cpu_set(cpuid, cpu_available_map);
> +		cpu_set(cpuid, cpu_possible_map);
> +		cpu_set(cpuid, cpu_present_at_boot);
> +		if (cpuid == boot_cpuid)
> +		cpu_set(cpuid, cpu_online_map);
> +
> +		/* set the secondary threads into the cpuid mask */
> +		for (ind=1; ind < cpu_threads; ind++) {
> +			cpuid++;
> +			if (cpuid >= NR_CPUS)
> +				continue;
> +			if (naca->smt_state) {
> +				cpu_set(cpuid, cpu_available_map);
> +				cpu_set(cpuid, cpu_present_at_boot);
> +			}
> +		}
> +#endif /* CONFIG_SMP */

Again I'd have the CONFIG_SMP cover more.  The whole while loop and the
of_node_put.

> +		cpuid++;
> +	}
> +	of_node_put(np);
> +
> +#ifdef CONFIG_HMT
> +	/* Only enable HMT on processors that provide support. */
> +	if (__is_processor(PV_PULSAR) ||
> +	 __is_processor(PV_ICESTAR) ||
> +	 __is_processor(PV_SSTAR)) {
> +
> +		for (ind = 0; ind < NR_CPUS; ind += 2) {
> +			if (!cpu_online(ind))
> +				continue;
> +			cpu_set(ind+1, cpu_possible_map);
> +		}
> +	}
> +#endif
> +	return;
> +}
> diff -Naur linux-2.6.8-rc2-org/arch/ppc64/kernel/prom.c linux-2.6.8-rc2-chg/arch/ppc64/kernel/prom.c
> --- linux-2.6.8-rc2-org/arch/ppc64/kernel/prom.c	2004-08-04 06:10:30.000000000 -0700
> +++ linux-2.6.8-rc2-chg/arch/ppc64/kernel/prom.c	2004-08-12 23:52:47.000000000 -0700
> @@ -939,13 +939,6 @@
>  			prom_getprop(node, "reg", &reg, sizeof(reg));
>  			lpaca[cpuid].hw_cpu_id = reg;
>
> -#ifdef CONFIG_SMP
> -			cpu_set(cpuid, RELOC(cpu_available_map));
> -			cpu_set(cpuid, RELOC(cpu_possible_map));
> -			cpu_set(cpuid, RELOC(cpu_present_at_boot));
> -			if (reg == 0)
> -				cpu_set(cpuid, RELOC(cpu_online_map));
> -#endif /* CONFIG_SMP */
>  			cpuid++;
>  		}
>  		return;
> @@ -1042,9 +1035,6 @@
>  #ifdef CONFIG_SMP
>  				/* Set the number of active processors. */
>  				_systemcfg->processorCount++;
> -				cpu_set(cpuid, RELOC(cpu_available_map));
> -				cpu_set(cpuid, RELOC(cpu_possible_map));
> -				cpu_set(cpuid, RELOC(cpu_present_at_boot));
>  #endif
>  			} else {
>  				prom_printf("... failed: %x\n", *acknowledge);
> @@ -1053,10 +1043,6 @@
>  #ifdef CONFIG_SMP
>  		else {
>  			prom_printf("%x : booting  cpu %s\n", cpuid, path);
> -			cpu_set(cpuid, RELOC(cpu_available_map));
> -			cpu_set(cpuid, RELOC(cpu_possible_map));
> -			cpu_set(cpuid, RELOC(cpu_online_map));
> -			cpu_set(cpuid, RELOC(cpu_present_at_boot));
>  		}
>  #endif
>  next:
> @@ -1069,13 +1055,6 @@
>  			lpaca[cpuid].hw_cpu_id = interrupt_server[i];
>  			prom_printf("%x : preparing thread ... ",
>  				    interrupt_server[i]);
> -			if (_naca->smt_state) {
> -				cpu_set(cpuid, RELOC(cpu_available_map));
> -				cpu_set(cpuid, RELOC(cpu_present_at_boot));
> -				prom_printf("available\n");
> -			} else {
> -				prom_printf("not available\n");
> -			}
>  		}
>  #endif
>  		cpuid++;
> @@ -1101,8 +1080,6 @@
>  						pir & 0x3ff;
>  				}
>  			}
> -/* 			cpu_set(i+1, cpu_online_map); */
> -			cpu_set(i+1, RELOC(cpu_possible_map));
>  		}
>  		_systemcfg->processorCount *= 2;
>  	} else {
> diff -Naur linux-2.6.8-rc2-org/arch/ppc64/kernel/setup.c linux-2.6.8-rc2-chg/arch/ppc64/kernel/setup.c
> --- linux-2.6.8-rc2-org/arch/ppc64/kernel/setup.c	2004-08-03 02:12:59.000000000 -0700
> +++ linux-2.6.8-rc2-chg/arch/ppc64/kernel/setup.c	2004-08-04 06:15:27.000000000 -0700
> @@ -76,6 +76,7 @@
>  extern void pseries_secondary_smp_init(unsigned long);
>  extern int  idle_setup(void);
>  extern void vpa_init(int cpu);
> +extern void cpumask_setup(void);

Could this go in a .h file somewhere?

>
>  unsigned long decr_overclock = 1;
>  unsigned long decr_overclock_proc0 = 1;
> @@ -229,6 +230,7 @@
>  		register_console(&udbg_console);
>  		__irq_offset_value = NUM_ISA_INTERRUPTS;
>  		finish_device_tree();
> +		cpumask_setup();
>  		chrp_init(r3, r4, r5, r6, r7);
>
>  #ifdef CONFIG_SMP
> @@ -251,6 +253,7 @@
>  #ifdef CONFIG_PPC_PMAC
>  	if (systemcfg->platform == PLATFORM_POWERMAC) {
>  		finish_device_tree();
> +		cpumask_setup();
>  		pmac_init(r3, r4, r5, r6, r7);
>  	}
>  #endif /* CONFIG_PPC_PMAC */

** Sent via the linuxppc64-dev mail list. See http://lists.linuxppc.org/





More information about the Linuxppc64-dev mailing list