Messages in this thread | | | Subject | Re: [PATCH v3 1/7] ACPI/PPTT: Add Processor Properties Topology Table parsing | From | Jeremy Linton <> | Date | Thu, 19 Oct 2017 09:24:12 -0500 |
| |
Hi,
On 10/19/2017 12:18 AM, Tomasz Nowicki wrote: > On 18.10.2017 19:30, Jeremy Linton wrote: >> On 10/18/2017 05:24 AM, Tomasz Nowicki wrote: >>> On 18.10.2017 07:39, Tomasz Nowicki wrote: >>>> On 17.10.2017 17:22, Jeremy Linton wrote: >>>>> On 10/17/2017 08:25 AM, Tomasz Nowicki wrote: >>>>>> On 12.10.2017 21:48, Jeremy Linton wrote: (trimming) >>>>>>> + if (*found != NULL) >>>>>>> + pr_err("Found duplicate cache level/type unable >>>>>>> to determine uniqueness\n"); >>> >>> Actually I still see this error messages in my dmesg. It is because >>> the following ThunderX2 per-core L1 and L2 cache hierarchy: >>> >>> Core >>> ------------------ >>> | | >>> | L1i ----- | >>> | | | >>> | ----L2 | >>> | | | >>> | L1d ----- | >>> | | >>> ------------------ >>> >>> In this case we have two paths which lead to L2 cache and hit above >>> case. Is it really error case? >> >> No, but its not deterministic unless we mark the node, which doesn't >> solve the problem of a table constructed like >> >> L1i->L2 (unified) >> L1d->L2 (unified) >> >> or various other structures which aren't disallowed by the spec and >> have non-deterministic real world meanings, anymore than constructing >> the table like: >> >> L1i >> Lid->L2(unified) >> >> which I tend to prefer because with a structuring like that it can be >> deterministic (and in a way actually represents the non-coherent >> behavior of (most?) ARM64 core's i-caches, as could be argued the >> first example if the allocation policies are varied between the L2 >> nodes). >> >> The really ugly bits here happen if you add another layer: >> >> L1i->L2i-L3 >> L1d------^ >> >> which is why I made that an error message, not including the fact that >> since the levels aren't tagged the numbering and meaning isn't clear. >> >> (the L1i in the above example might be better called an L0i to avoid >> throwing off the reset of the hierarchy numbering, also so it could be >> ignored). >> >> Summary: >> >> I'm not at all happy with this specification's attempt to leave out >> pieces of information which make parsing things more deterministic. In >> this case I'm happy to demote the message level, but not remove it >> entirely but I do think the obvious case you list shouldn't be the >> default one. >> >> Lastly: >> >> I'm assuming the final result is that the table is actually being >> parsed correctly despite the ugly message? > > Indeed, the ThunderX2 PPTT table is being parsed so that topology shown > in lstopo and lscpu is correct.
Great.
Also, I think this is a better change:
if ((*found != NULL) && (*found != cache)) pr_err("Found duplicate cache level/type unable to determine uniqueness\n");
Which if its a duplicate node/type at the given level the message is just suppressed. It will of course still trigger in cases like:
L1d->L2 l1i->L2
or other odd cases.
| |