Messages in this thread | | | Date | Tue, 5 May 2009 18:47:33 +0200 | From | Andreas Herrmann <> | Subject | Re: [PATCH 0/3] x86: adapt CPU topology detection for AMD Magny-Cours |
| |
On Tue, May 05, 2009 at 05:31:59PM +0200, Andi Kleen wrote: > On Tue, May 05, 2009 at 04:40:27PM +0200, Andreas Herrmann wrote: > > I think the general problem with your patchset is that it seems > more like a solution in search of a problem, instead of the proper other > way around.
<snip>
> > > First I must say it's unclear to me if CPU topology is really generally > > > useful to export to the user. > > > > I think it is useful. > > You forgot to state for what?
You are kidding, aren't you? Isn't this obvious? Why shouldn't a user be interested in stuff like core_siblings and thread_siblings? Maybe a user want to make scheduling decisions based on that and pin tasks accordingly?
<snip>
> > (a) Are you saying that users have to check NUMA distances when they > > want to pin tasks on certain CPUs? > > CPU == core ? No you just bind to that CPU. Was that a trick question?
Of course that was no trick question -- at most a stupid typo. (Sometimes in Linux CPU == core.) So, no, sorry I meant "certain cores". (And I meant not pinning to one core but to a set of cores).
<snip>
> > representing entire CPU topology in sysfs makes this obvious. > > You didn't do that.
I partially did that and mentioned what else might be needed, see
http://marc.info/?l=linux-kernel&m=124145920830040
<snip>
> > Yes, it's a new "special case" which can't be expressed in other ways. > > You seem to assume that it's obviously useful to express. Perhaps > I'm dumb, but can you explain for what? > > > SLIT and SRAT are not sufficient. > > > > The kernel > > Which part of the kernel?
I provided this info in my first reply to you. Here it is again:
"The kernel needs to know this when accessing processor configuration space, when accessing shared MSRs or for counting northbridge specific events."
To translate this for you. Potential users are - EDAC ;-) - other MCA related stuff (e.g. L3 cache index disable) - performance monitoring - most probably everything that accesses processor configuration space and shared MSRs
I think it is best to store this information centrally instead of letting each component figuring out which cores are on the same node.
<snip>
> The general problem is that you're trying to stretch an old ad-hoc > hack (siblings etc. in /proc/cpuinfo) to a complicated new graph structure > and the interface is just not up to that.
You didn't read all my mails regarding this topic. The patches fixup sibling information for Magny-Cours. This info is not only exposed to /proc/cpuinfo but also with cpu-topology information in sysfs. I don't see why /sys/devices/system/cpu/cpuX/topology is an old ad-hoc hack.
> As a exercise you can try to write a user space program that uses > these fields to query the topology. It will be a mess.
You shouldn't try to use /proc/cpuinfo for that but instead look up topology in /sys/devices/system/cpu/cpuX/topology. BTW, I have already such a script and I am using it.
> We went through the same with the caches. Initially /proc/cpuinfo > tried to express the caches. At some point it just became too messy > and cpuinfo now only gives a very simplified "legacy" field and > the real information is in /sys. It went into /sys because there > was a clear use case. If there's a clear use case for exposing > complex CPU topology (so far that's still an open question > due to lack of good examples) then also a new proper non hacky > interface in /sys would make sense.
CPU topology is already in /sys.
Regards, Andreas
-- Operating | Advanced Micro Devices GmbH System | Karl-Hammerschmidt-Str. 34, 85609 Dornach b. München, Germany Research | Geschäftsführer: Thomas M. McCoy, Giuliano Meroni Center | Sitz: Dornach, Gemeinde Aschheim, Landkreis München (OSRC) | Registergericht München, HRB Nr. 43632
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |