svn commit: r208165 - in head/sys: kern mips/conf mips/include mips/mips mips/rmi mips/rmi/dev/xlr

[Jayachandran C.]

On Mon, Aug 9, 2010 at 5:35 PM, Attilio Rao <attilio at freebsd.org> wrote:
> 2010/8/9 Jayachandran C. <c.jayachandran at gmail.com>:
>> On Mon, Aug 9, 2010 at 5:31 AM, Attilio Rao <attilio at freebsd.org> wrote:
>>> 2010/5/16 Randall Stewart <rrs at freebsd.org>:
>>>> Author: rrs
>>>> Date: Sun May 16 19:43:48 2010
>>>> New Revision: 208165
>>>> URL: http://svn.freebsd.org/changeset/base/208165
>>>>
>>>> Log:
>>>>  This pushes all of JC's patches that I have in place. I
>>>>  am now able to run 32 cores ok.. but I still will hang
>>>>  on buildworld with a NFS problem. I suspect I am missing
>>>>  a patch for the netlogic rge driver.
>>>>
>>>>  JC check and see if I am missing anything except your
>>>>  core-mask changes
>>>
>>>
>>>> Modified: head/sys/kern/subr_smp.c
>>>> ==============================================================================
>>>> --- head/sys/kern/subr_smp.c    Sun May 16 19:25:56 2010        (r208164)
>>>> +++ head/sys/kern/subr_smp.c    Sun May 16 19:43:48 2010        (r208165)
>>>> @@ -503,7 +503,10 @@ smp_topo_none(void)
>>>>        top = &group[0];
>>>>        top->cg_parent = NULL;
>>>>        top->cg_child = NULL;
>>>> -       top->cg_mask = (1 << mp_ncpus) - 1;
>>>> +       if (mp_ncpus == sizeof(top->cg_mask) * 8)
>>>> +               top->cg_mask = -1;
>>>> +       else
>>>> +               top->cg_mask = (1 << mp_ncpus) - 1;
>>>>        top->cg_count = mp_ncpus;
>>>>        top->cg_children = 0;
>>>>        top->cg_level = CG_SHARE_NONE;
>>>>
>>>
>>> ... and this is why I particulary hate big commits with complete lack
>>> of technical details.
>>>
>>> This particulary chunk was supposed to fix a nasty and completely MI
>>> bug that some users have already met (kern/148698). The complete lack
>>> of details didn't help in identify the issue neither that it was a
>>> valuable fix.
>>>
>>> The fix is, however, improper (there is no clear relationship between
>>> the multiplication and why that happens) thus I would rather use what
>>> Joe has reported in the PR.
>>
>>
>> I was not aware of the PR when I sent this changes to rrs@, and this
>> went as a part of MIPS SMP support for XLR which has 32 CPUs
>>
>> But I'm not sure that the current change is correct, cg_mask is of
>> type cpumask_t and I don't think it is guaranteed to be 32 bit (as it
>> is a machine dependent type).
>
> Actually the 32 bits limit is well aware and acknowledged in cpumask_t.
> While it is true that it should be an 'opaque' type, in reality it is
> not. The maximum limit of 32 CPUs is a reality due to cpumask_t on all
> our architectures.
> That is why we are going to use cpuset_t for dealing with CPUs numbers
> (which is really opaque, at same extent, and is not bound to any
> size).

In my opinion, your changes added another pitfall for the person who
tries to make the cpumask_t 64 bit to support more cpus, which really
could have been avoided.

But if cpumask_t is going to be changed to cpuset_t soon, I guess we
are fine  :)

JC.