Re: [PATCH 2/2] edac: add support for Amazon's Annapurna Labs EDAC

[James Morse <james.morse@arm.com>]

Hi Hawa,

On 13/06/2019 18:05, James Morse wrote:
> On 11/06/2019 20:56, Hawa, Hanna wrote:
>> James Morse wrote:
>>> Hawa, Hanna wrote:
>>>> +        if (cluster != last_cluster) {
>>>> +            smp_call_function_single(cpu, al_a57_edac_l2merrsr,
>>>> +                         edac_dev, 0);
>>>> +            last_cluster = cluster;
>>>> +        }
>>> Here you depend on the CPUs being listed in cluster-order in the DT. I'm fairly sure the
>>> numbering is arbitrary: On my Juno 0,3,4,5 are the A53 cluster, and 1,2 are the A57
>>> cluster.
>>>
>>> If 1,3,5 were cluster-a and 2,4,6 were cluster-b, you would end up calling
>>> al_a57_edac_l2merrsr() for each cpu. As you don't wait, they could race.
>>>
>>> If you can get a cpu-mask for each cluster, smp_call_function_any() would to the
>>> pick-one-online-cpu work for you.

>> Again, I rely on that it's alpine SoC specific driver.

An example of where this goes wrong is kexec:
If you offline CPU0, then kexec, the new kernel will start up on the lowest numbered
online CPU, which won't be zero. But the new kernel will call it CPU0.

Kdump is even better, as it starts up on whichever CPU called panic(), and calls it CPU0.


Thanks,

James


>> How can I get cpu-mask for each cluster? from DT?

> Its not cluster you want, its the L2. Cacheinfo has this for online CPUs, and you're
> already holding the cpus_read_lock().