lkml.org 
[lkml]   [2017]   [Oct]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v3 1/7] ACPI/PPTT: Add Processor Properties Topology Table parsing
From
Date
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.





\
 
 \ /
  Last update: 2017-10-22 17:26    [W:0.092 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site