[Skiboot] [PATCH] fix lock error when BT IRQ preempt BT timer

Vasant Hegde hegdevasant at linux.vnet.ibm.com
Wed Jan 13 20:55:52 AEDT 2021


On 1/12/21 11:13 AM, Oliver O'Halloran wrote:
> On Tue, Jan 12, 2021 at 4:24 PM Vasant Hegde
> <hegdevasant at linux.vnet.ibm.com> wrote:
>>
>> On 1/11/21 3:26 PM, DD Aric li wrote:
>>> Hi Vasant:
>>> I wonder if every core will buffer infight_bt_msg in it own cache.
>>> If that true, can the lock ensure that the value of "infight_bt_msg" read
>>> by every thread is the latest?
>>
>> I think lock will take care of it. We don't need volatile.
> 
> Just FYI only one half of this conversation is going to the mailing list.

Thanks! Looks like accidentally I marked `lixgemail at gmail.com` ID to filter list.
I have fixed it. Now we should get all emails to mailing list.


> 
> Anyway, the kernel docs have a decent overview of why the locking
> primitives should be used rather than volatile:
> https://www.kernel.org/doc/Documentation/process/volatile-considered-harmful.rst
> The locks in skiboot were designed along similar lines to the
> spinlocks in the kernel so most of the advice there applies to
> skiboot. If you're seeing crashes which seem to be fixed by adding
> "volatile" then I'd say that some code which should be taking the lock
> isn't. There's also some potential pitfalls around the ordering of
> MMIO operations vs unlock() since the lwsync() we do on unlock doesn't
> enforce ordering between MMIO and regular memory ops, but I'd rule out
> the missing lock before going down that rabbit hole.

Thanks for the details.

-Vasant


More information about the Skiboot mailing list