On Tue, 2007-04-10 at 11:59 -0500, Milton Miller wrote: > On Apr 10, 2007, at 2:06 AM, Michael Ellerman wrote: > > > On Mon, 2007-04-09 at 14:27 +0530, Mohan Kumar M wrote: > >> On Wed, Mar 07, 2007 at 11:52:32AM +0100, Michael Ellerman wrote: > >>> There's already maxcpus in init/main.c, that would probably be > >>> better, > >>> though still ugly. > >>> > >> Based on Mike's suggestions, I modified the patch. The attached patch > >> refers max_cpus variable to check whether the kernel is booted with > >> maxcpus=1 parameter and if maxcpus=1 is specified the patch assigns > >> only > >> the current boot cpu to be the default distribution server. > > > > So the core of the problem is that if we haven't onlined all cpus then > > we can't use the default_distrib_server value given to us by firmware, > > because some of the cpus in that queue won't be online. > > > > We can detect this situation by comparing the number of cpus that are > > online vs the number that are present (not possible). This might even > > work if you boot with maxcpus=1 and then hotplug the rest in. > > > > How about this: > > > > Index: powerpc/arch/powerpc/platforms/pseries/xics.c > > =================================================================== > > --- powerpc.orig/arch/powerpc/platforms/pseries/xics.c > > +++ powerpc/arch/powerpc/platforms/pseries/xics.c > > @@ -167,7 +167,10 @@ static int get_irq_server(unsigned int v > > return default_server; > > > > if (cpus_equal(cpumask, CPU_MASK_ALL)) { > > - server = default_distrib_server; > > + if (num_online_cpus() == num_present_cpus()) > > + server = default_distrib_server; > > + else > > + server = default_server; > > } else { > > cpus_and(tmp, cpu_online_map, cpumask); > > This means we are doing population counts of two masks, when we really > just care that they are the same. How about using > cpus_equal(cpu_online_mask, cpu_present_mask)? Yep that's sensible. > > > > @@ -415,7 +418,10 @@ static void xics_set_affinity(unsigned i > > > > /* For the moment only implement delivery to all cpus or one cpu */ > > if (cpus_equal(cpumask, CPU_MASK_ALL)) { > > - newmask = default_distrib_server; > > + if (num_online_cpus() == num_present_cpus()) > > + newmask = default_distrib_server; > > + else > > + newmask = default_server; > > } else { > > cpus_and(tmp, cpu_online_map, cpumask); > > if (cpus_empty(tmp)) > > This shows how close these two functions are. The difference is > what happens when cpus_empty is true -- we default in one and > ignore in the other. > > How about adding another arg to get_server, that says to fail > or default for the empty case, with failure being -1? > > I'll try to code this up, but it might be a day or two until i get > the time. Yeah it annoyed me that the code was so similar, but it is a small but important semantic difference. I'll let you code it up, you know that code better than me. cheers -- Michael Ellerman OzLabs, IBM Australia Development Lab wwweb: http://michael.ellerman.id.au phone: +61 2 6212 1183 (tie line 70 21183) We do not inherit the earth from our ancestors, we borrow it from our children. - S.M.A.R.T Person
Attachment:
signature.asc
Description: This is a digitally signed message part
_______________________________________________ fastboot mailing list fastboot@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/fastboot