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

[Oliver O'Halloran]

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.

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.

Oliver