[PATCH v2 3/3] powerpc/spinlocks: Fix oops in shared-processor spinlocks
Christopher M Riedl
cmr at informatik.wtf
Tue Aug 6 22:31:40 AEST 2019
> On August 6, 2019 at 7:14 AM Michael Ellerman <mpe at ellerman.id.au> wrote:
>
>
> Christopher M Riedl <cmr at informatik.wtf> writes:
> >> On August 2, 2019 at 6:38 AM Michael Ellerman <mpe at ellerman.id.au> wrote:
> >> "Christopher M. Riedl" <cmr at informatik.wtf> writes:
> >>
> >> This leaves us with a double test of is_shared_processor() doesn't it?
> >
> > Yep, and that's no good. Hmm, executing the barrier() in the non-shared-processor
> > case probably hurts performance here?
>
> It's only a "compiler barrier", so it shouldn't generate any code.
>
> But it does have the effect of telling the compiler it can't optimise
> across that barrier, which can be important.
>
> In those spin loops all we're doing is checking lock->slock which is
> already marked volatile in the definition of arch_spinlock_t, so the
> extra barrier shouldn't really make any difference.
>
> But still the current code doesn't have a barrier() there, so we should
> make sure we don't introduce one as part of this refactor.
Thank you for taking the time to explain this. I have some more reading to
do about compiler-barriers it seems :)
>
> So I think you just want to change the call to spin_yield() above to
> splpar_spin_yield(), which avoids the double check, and also avoids the
> barrier() in the SPLPAR=n case.
>
> And then arch_spin_relax() calls spin_yield() etc.
I submitted a v3 before your reply with this change already - figured this
is the best way to avoid the double check and maintain legacy behavior.
>
> cheers
More information about the Linuxppc-dev
mailing list