Alex Williamson
2007-Mar-02 20:39 UTC
[Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
Hi Ewan, There are a couple problems with this patch for non-x86 (and maybe even on x86), see below: On Tue, 2007-02-27 at 01:56 +0000, Xen staging patchbot-unstable wrote:> diff -r e7b2a282c9e7 -r 50e0616fd012 tools/python/xen/xend/XendNode.py > --- a/tools/python/xen/xend/XendNode.py Mon Feb 26 17:20:36 2007 +0000 > +++ b/tools/python/xen/xend/XendNode.py Tue Feb 27 00:37:27 2007 +0000 > @@ -81,7 +81,7 @@ class XendNode: > for cpu_uuid, cpu in saved_cpus.items(): > self.cpus[cpu_uuid] = cpu > > - # verify we have enough cpus here > + cpuinfo = parse_proc_cpuinfo() > physinfo = self.physinfo_dict() > cpu_count = physinfo[''nr_cpus''] > cpu_features = physinfo[''hw_caps''] > @@ -91,12 +91,23 @@ class XendNode: > if cpu_count != len(self.cpus): > self.cpus = {} > for i in range(cpu_count): > - cpu_uuid = uuid.createString() > - cpu_info = {''uuid'': cpu_uuid, > - ''host'': self.uuid, > - ''number'': i, > - ''features'': cpu_features} > - self.cpus[cpu_uuid] = cpu_info > + u = uuid.createString() > + self.cpus[u] = {''uuid'': u, ''number'': i } > + > + for u in self.cpus.keys(): > + log.error(self.cpus[u]) > + number = self.cpus[u][''number''] > + log.error(number) > + log.error(cpuinfo) > + self.cpus[u].update( > + { ''host'' : self.uuid, > + ''features'' : cpu_features, > + ''speed'' : int(float(cpuinfo[number][''cpu MHz''])), > + ''vendor'' : cpuinfo[number][''vendor_id''], > + ''modelname'': cpuinfo[number][''model name''], > + ''stepping'' : cpuinfo[number][''stepping''], > + ''flags'' : cpuinfo[number][''flags''], > + })On ia64, dom0 doesn''t automatically get vcpus for each physical cpu, so the first problem is that we''re not going to have a /proc/cpuinfo entry for every cpu in self.cpus.keys. I think it''s likely x86 could run into this problem too if a cpu was hotplugged or booted with the dom0_max_vcpus options. The second problem is that /proc/cpuinfo fields are very architecture specific. I''d suggest importing arch and having separate cases for x86, ia64, and powerpc. For ia64, think the most appropriate mapping would be: self.cpus[u].update( { ''host'' : self.uuid, ''features'' : cpu_features, ''speed'' : int(float(cpuinfo[0][''cpu MHz''])), ''vendor'' : cpuinfo[0][''vendor''], ''modelname'': cpuinfo[0][''family''], ''stepping'' : cpuinfo[0][''model''], ''flags'' : cpuinfo[0][''features''], }) Hollis or Jimi might be able to chime in with identifiers that would work on powerpc. Thanks, Alex -- Alex Williamson HP Open Source & Linux Org. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Mar-02 20:46 UTC
Re: [Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On 2/3/07 20:39, "Alex Williamson" <alex.williamson@hp.com> wrote:> On ia64, dom0 doesn''t automatically get vcpus for each physical cpu, > so the first problem is that we''re not going to have a /proc/cpuinfo > entry for every cpu in self.cpus.keys. I think it''s likely x86 could > run into this problem too if a cpu was hotplugged or booted with the > dom0_max_vcpus options.We have indeed hit this problem and I put a patch in this afternoon to duplicate cpu0''s info for any non-existent cpu. Given that the dom0 cpus could be migrating around on arbitrary physical cpus (even across the multiple CPUID invocations that the kernel will have made to build the information for a single ''cpu'' in /proc!) this is fine -- x86 multiprocessor systems are supposed to be symmetric (homogeneous CPUs down to the same stepping in some cases) anyway. -- Keir> The second problem is that /proc/cpuinfo fields are very architecture > specific. I''d suggest importing arch and having separate cases for x86, > ia64, and powerpc. For ia64, think the most appropriate mapping would > be:_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Mar-02 20:47 UTC
Re: [Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On Fri, Mar 02, 2007 at 01:39:46PM -0700, Alex Williamson wrote:> The second problem is that /proc/cpuinfo fields are very architecture > specific. I''d suggest importing arch and having separate cases for x86, > ia64, and powerpc. For ia64, think the most appropriate mapping would > be:And that there''s other OS''s that don''t stuff things in /proc at all. Sigh. john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
John Levon
2007-Mar-02 20:54 UTC
Re: [Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On Fri, Mar 02, 2007 at 08:47:21PM +0000, John Levon wrote:> > The second problem is that /proc/cpuinfo fields are very architecture > > specific. I''d suggest importing arch and having separate cases for x86, > > ia64, and powerpc. For ia64, think the most appropriate mapping would > > be: > > And that there''s other OS''s that don''t stuff things in /proc at all. > Sigh.Talking of which, XendMonitor seems to re-implement libxenstat''s VBD/VIF code in Python. Couldn''t we use one or the other? regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Alex Williamson
2007-Mar-02 20:55 UTC
Re: [Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On Fri, 2007-03-02 at 20:46 +0000, Keir Fraser wrote:> On 2/3/07 20:39, "Alex Williamson" <alex.williamson@hp.com> wrote: > > > On ia64, dom0 doesn''t automatically get vcpus for each physical cpu, > > so the first problem is that we''re not going to have a /proc/cpuinfo > > entry for every cpu in self.cpus.keys. I think it''s likely x86 could > > run into this problem too if a cpu was hotplugged or booted with the > > dom0_max_vcpus options. > > We have indeed hit this problem and I put a patch in this afternoon to > duplicate cpu0''s info for any non-existent cpu. Given that the dom0 cpus > could be migrating around on arbitrary physical cpus (even across the > multiple CPUID invocations that the kernel will have made to build the > information for a single ''cpu'' in /proc!) this is fine -- x86 multiprocessor > systems are supposed to be symmetric (homogeneous CPUs down to the same > stepping in some cases) anyway.Cool, I see it in staging. This is the approach I used to work around the problem temporarily, but what happens if cpu0 is hot un-plugged? ISTR x86 Linux doesn''t support cpu0 hotplug, but on ia64 we can hotplug cpu0. I''d guess powerpc could too. Thanks, Alex -- Alex Williamson HP Open Source & Linux Org. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2007-Mar-02 20:58 UTC
Re: [Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On 2/3/07 20:55, "Alex Williamson" <alex.williamson@hp.com> wrote:>> We have indeed hit this problem and I put a patch in this afternoon to >> duplicate cpu0''s info for any non-existent cpu. Given that the dom0 cpus >> could be migrating around on arbitrary physical cpus (even across the >> multiple CPUID invocations that the kernel will have made to build the >> information for a single ''cpu'' in /proc!) this is fine -- x86 multiprocessor >> systems are supposed to be symmetric (homogeneous CPUs down to the same >> stepping in some cases) anyway. > > Cool, I see it in staging. This is the approach I used to work > around the problem temporarily, but what happens if cpu0 is hot > un-plugged? ISTR x86 Linux doesn''t support cpu0 hotplug, but on ia64 we > can hotplug cpu0. I''d guess powerpc could too. Thanks,Then picking another number than zero would be a good idea. :-) -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2007-Mar-02 21:01 UTC
[Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
On Fri, 2007-03-02 at 13:39 -0700, Alex Williamson wrote:> > On ia64, dom0 doesn''t automatically get vcpus for each physical cpu, > so the first problem is that we''re not going to have a /proc/cpuinfo > entry for every cpu in self.cpus.keys. I think it''s likely x86 could > run into this problem too if a cpu was hotplugged or booted with the > dom0_max_vcpus options.Not sure if this affects us.> The second problem is that /proc/cpuinfo fields are very architecture > specific. I''d suggest importing arch and having separate cases for x86, > ia64, and powerpc. For ia64, think the most appropriate mapping would > be: > > self.cpus[u].update( > { ''host'' : self.uuid, > ''features'' : cpu_features, > ''speed'' : int(float(cpuinfo[0][''cpu MHz''])), > ''vendor'' : cpuinfo[0][''vendor''], > ''modelname'': cpuinfo[0][''family''], > ''stepping'' : cpuinfo[0][''model''], > ''flags'' : cpuinfo[0][''features''], > })This absolutely does. Ewan, what are you trying to do here, and could you please revert it until we figure out a solution that will work? -- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Hollis Blanchard
2007-Mar-07 17:54 UTC
[Xen-devel] Re: [Xen-staging] [xen-unstable] Added some more fields to host_cpu.
Hi Keir, I see the patch you''ve committed for this issue (http://xenbits2.xensource.com/staging/xen-unstable.hg?rev/ae203b55e7c8), but my concern goes a littler further. What will this data be used for? Will higher-level tools expect to find "stepping" and break if they don''t? Will XenSource''s management tools (which I assume will make use of this data) function properly in this case? I agree that hardware-specific data needs to be visible somehow to higher-level tools, but it makes me nervous... (I don''t have any particular objection to your fix; just voicing my concerns.) On Fri, 2007-03-02 at 13:39 -0700, Alex Williamson wrote:> Hi Ewan, > > There are a couple problems with this patch for non-x86 (and maybe > even on x86), see below: > > On Tue, 2007-02-27 at 01:56 +0000, Xen staging patchbot-unstable wrote: > > diff -r e7b2a282c9e7 -r 50e0616fd012 tools/python/xen/xend/XendNode.py > > --- a/tools/python/xen/xend/XendNode.py Mon Feb 26 17:20:36 2007 +0000 > > +++ b/tools/python/xen/xend/XendNode.py Tue Feb 27 00:37:27 2007 +0000 > > @@ -81,7 +81,7 @@ class XendNode: > > for cpu_uuid, cpu in saved_cpus.items(): > > self.cpus[cpu_uuid] = cpu > > > > - # verify we have enough cpus here > > + cpuinfo = parse_proc_cpuinfo() > > physinfo = self.physinfo_dict() > > cpu_count = physinfo[''nr_cpus''] > > cpu_features = physinfo[''hw_caps''] > > @@ -91,12 +91,23 @@ class XendNode: > > if cpu_count != len(self.cpus): > > self.cpus = {} > > for i in range(cpu_count): > > - cpu_uuid = uuid.createString() > > - cpu_info = {''uuid'': cpu_uuid, > > - ''host'': self.uuid, > > - ''number'': i, > > - ''features'': cpu_features} > > - self.cpus[cpu_uuid] = cpu_info > > + u = uuid.createString() > > + self.cpus[u] = {''uuid'': u, ''number'': i } > > + > > + for u in self.cpus.keys(): > > + log.error(self.cpus[u]) > > + number = self.cpus[u][''number''] > > + log.error(number) > > + log.error(cpuinfo) > > + self.cpus[u].update( > > + { ''host'' : self.uuid, > > + ''features'' : cpu_features, > > + ''speed'' : int(float(cpuinfo[number][''cpu MHz''])), > > + ''vendor'' : cpuinfo[number][''vendor_id''], > > + ''modelname'': cpuinfo[number][''model name''], > > + ''stepping'' : cpuinfo[number][''stepping''], > > + ''flags'' : cpuinfo[number][''flags''], > > + }) > > On ia64, dom0 doesn''t automatically get vcpus for each physical cpu, > so the first problem is that we''re not going to have a /proc/cpuinfo > entry for every cpu in self.cpus.keys. I think it''s likely x86 could > run into this problem too if a cpu was hotplugged or booted with the > dom0_max_vcpus options. > > The second problem is that /proc/cpuinfo fields are very architecture > specific. I''d suggest importing arch and having separate cases for x86, > ia64, and powerpc. For ia64, think the most appropriate mapping would > be: > > self.cpus[u].update( > { ''host'' : self.uuid, > ''features'' : cpu_features, > ''speed'' : int(float(cpuinfo[0][''cpu MHz''])), > ''vendor'' : cpuinfo[0][''vendor''], > ''modelname'': cpuinfo[0][''family''], > ''stepping'' : cpuinfo[0][''model''], > ''flags'' : cpuinfo[0][''features''], > }) > > Hollis or Jimi might be able to chime in with identifiers that would > work on powerpc. Thanks, > > Alex-- Hollis Blanchard IBM Linux Technology Center _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel