[RFC PATCH 05/17] ARM: kernel: save/restore kernel IF

[Russell King - ARM Linux]

On Sat, Jul 09, 2011 at 09:38:15AM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 07, 2011 at 04:50:18PM +0100, Lorenzo Pieralisi wrote:
> > +static int late_init(void)
> > +{
> > +	int rc;
> > +	struct sr_cluster *cluster;
> > +	int cluster_index, cpu_index = sr_platform_get_cpu_index();
> 
> Stop this madness, and use the standard linux APIs like smp_processor_id
> here.  It might actually help you find bugs, like the fact that you're
> in a preemptible context here, and so could be rescheduled onto any other
> CPU in the system _after_ you've read the MPIDR register.
> 
> > +
> > +	cluster_index = sr_platform_get_cluster_index();
> > +	cluster = main_table.cluster_table + cluster_index;
> > +	main_table.os_mmu_context[cluster_index][cpu_index] =
> > +				current->active_mm->pgd;
> > +	cpu_switch_mm(main_table.fw_mmu_context, current->active_mm);
> > +	rc = sr_platform_init();
> > +	cpu_switch_mm(main_table.os_mmu_context[cluster_index][cpu_index],
> > +				current->active_mm);
> 
> CPU numbers are unique in the system, why do you need a 'cluster_index'
> to save this?  In fact why do you even need to save it in a structure
> at all?
> 
> Plus, "cluster" is not used, please get rid of it.

Oh, that's another thing.  This thread is about introducing idle support
not cluster support.  Cluster support is surely something different (esp.
as it seems from the above code that you're trying to support several
clusters of MPCore CPUs, each with physical 0-N CPU numbers.)

Cluster support should be an entirely separate patch series.

If that is not what this cluster stuff in this patch is about, then it's
just written badly and reinforces the need for it to be rewritten - in
that case there's no need for a 2D array.

In any case, smp_processor_id() will be (and must be) unique in any given
running kernel across all CPUs, even if you have clusters of N CPUs all
physically numbered 0-N.