Date: Mon, 9 Aug 2010 19:56:49 +0530 From: "Jayachandran C." <c.jayachandran@gmail.com> To: Attilio Rao <attilio@freebsd.org> Cc: src-committers@freebsd.org, John Baldwin <jhb@freebsd.org>, "Jayachandran C." <jchandra@freebsd.org>, svn-src-all@freebsd.org, Joe Landers <jlanders@vmware.com>, Randall Stewart <rrs@freebsd.org>, sbruno@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r208165 - in head/sys: kern mips/conf mips/include mips/mips mips/rmi mips/rmi/dev/xlr Message-ID: <AANLkTin9tn%2BOFOkQSYSewtgVQrDNoa2gE2SqPxA73cih@mail.gmail.com> In-Reply-To: <AANLkTim_pTQgquRRuz4G=jC5G0Amc%2ByF6VeDcg-cpTCX@mail.gmail.com> References: <201005161943.o4GJhnTo096839@svn.freebsd.org> <AANLkTikAarFgbxgGu-8XG7gh6VidPoVGwva54NN4rcRF@mail.gmail.com> <AANLkTi=jkUiXStPV3Gq206GgmctM9%2B1ofbNWuh5CSeWw@mail.gmail.com> <AANLkTim_pTQgquRRuz4G=jC5G0Amc%2ByF6VeDcg-cpTCX@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Aug 9, 2010 at 5:35 PM, Attilio Rao <attilio@freebsd.org> wrote: > 2010/8/9 Jayachandran C. <c.jayachandran@gmail.com>: >> On Mon, Aug 9, 2010 at 5:31 AM, Attilio Rao <attilio@freebsd.org> wrote: >>> 2010/5/16 Randall Stewart <rrs@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.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTin9tn%2BOFOkQSYSewtgVQrDNoa2gE2SqPxA73cih>