From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chao Peng Subject: Re: [PATCH v7 01/14] x86: add socket_to_cpumask Date: Tue, 19 May 2015 17:51:20 +0800 Message-ID: <20150519095120.GG8239@pengc-linux.bj.intel.com> References: <1431075415-21917-1-git-send-email-chao.p.peng@linux.intel.com> <1431075415-21917-2-git-send-email-chao.p.peng@linux.intel.com> <555A0384020000780007B2A7@mail.emea.novell.com> <20150519061224.GA8239@pengc-linux.bj.intel.com> <555AF441020000780007B6A1@mail.emea.novell.com> <20150519064721.GB8239@pengc-linux.bj.intel.com> <555AF9B4020000780007B6E5@mail.emea.novell.com> <20150519071039.GC8239@pengc-linux.bj.intel.com> <555B0309020000780007B722@mail.emea.novell.com> Reply-To: Chao Peng Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Content-Disposition: inline In-Reply-To: <555B0309020000780007B722@mail.emea.novell.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Jan Beulich Cc: wei.liu2@citrix.com, Ian.Campbell@citrix.com, stefano.stabellini@eu.citrix.com, andrew.cooper3@citrix.com, Ian.Jackson@eu.citrix.com, xen-devel@lists.xen.org, will.auld@intel.com, keir@xen.org, dgdegra@tycho.nsa.gov List-Id: xen-devel@lists.xenproject.org On Tue, May 19, 2015 at 08:31:53AM +0100, Jan Beulich wrote: > >>> On 19.05.15 at 09:10, wrote: > > On Tue, May 19, 2015 at 07:52:04AM +0100, Jan Beulich wrote: > >> >>> On 19.05.15 at 08:47, wrote: > >> > On Tue, May 19, 2015 at 07:28:49AM +0100, Jan Beulich wrote: > >> >> >>> On 19.05.15 at 08:12, wrote: > >> >> > On Mon, May 18, 2015 at 02:21:40PM +0100, Jan Beulich wrote: > >> >> >> >>> On 08.05.15 at 10:56, wrote: > >> >> >> > @@ -112,6 +115,8 @@ static int __devinit MP_processor_info_x(struct > >> > mpc_config_processor *m, > >> >> >> > { > >> >> >> > int ver, apicid, cpu = 0; > >> >> >> > > >> >> >> > + total_cpus++; > >> >> >> > + > >> >> >> > if (!(m->mpc_cpuflag & CPU_ENABLED)) { > >> >> >> > if (!hotplug) > >> >> >> > ++disabled_cpus; > >> >> >> > >> >> >> Is there a reason you can't use disabled_cpus and avoid adding yet > >> >> >> another variable? > >> >> > > >> >> > The problem is not with disabled_cpus but with num_processors, which > >> >> > does not keep the original detected cpus in current code. > >> >> > Hence 'total_cpus = disabled_cpus + num_processors' may not be correct > >> >> > in some cases. > >> >> > >> >> Please be more specific about when this is a problem (I do note that > >> >> I'm aware that the equation will not always hold, but during my > >> >> inspection while reviewing your change I didn't see that this would > >> >> ever become problematic). > >> > > >> > What I really need is the original cpu count enumerated from MADT. If > >> > not introduce total_cpus then the only way getting it AFAICS is > >> > 'disabled_cpus + num_processors'. > >> > > >> > The problem is that MP_processor_info_x() have some earlier returns > >> > before increasing num_processors. In those cases, the cpu detected will > >> > neither counted to disabled_cpus nor num_processors, which means > >> > 'disabled_cpus + num_processors' is potentially small than what I need. > >> > >> As said - I understand this. But you still fail to explain under what > >> (realistic, i.e. other than someone bogusly setting NR_CPUS=1) > >> conditions this ends up being a problem. > > > > As we calculate nr_sockets with: > > > > nr_sockets = total_cpus / _cpus_per_socket__ > > > > If the calculated total_cpus is smaller than the actual cpu count on the > > hardware, then the nr_sockets is also potentially smaller than the > > actual socket count on the hardware. This is not the expectation. > > Sure - but you still don't say what is going to go wrong. Remember, > when I asked you to change to the total count I gave an explicit > example: Use of "nosmp" would have yielded a zero nr_sockets in > the earlier code. Yet with the sum of num_processors and > disabled_cpus this can't happen anymore afaict. "nosmp" only has side effect on max_cpus and nr_cpu_ids, but they are never used at all when calculating nr_sockets. So I can't see any reason why with "num_processors + disabled_cpus" the nr_sockets would not be zero, I think this is a bug that I should fix in nosmp case. > Hence I'm looking > forward to you detailing the conditions under which you would see > an issue without introducing total_cpus. As said before, with "num_processors + disabled_cpus" I may get a smaller nr_sockets than the machine actually has. This is my exact problem: I may miss enumerating some CAT-enabled sockets. While the assumption is that I will follow your suggestion to make nr_socket >= the socket count that the machine actually has. Chao