Re: spinlock recursion when running q800 emulation in qemu

[Michael Schmitz <schmitzmic@gmail.com>]

Hi Finn,

On 11/03/24 21:35, Finn Thain wrote:
> On Mon, 11 Mar 2024, Michael Schmitz wrote:
>   
>> Looping over 178 boots (using init=/sbin/reboot), I see eight of the
>> spinlock recursion messages in ARAnyM on my old PowerBook G4:
>>
>> BUG: spinlock recursion on CPU#0, swapper/1
>> BUG: spinlock recursion on CPU#0, swapper/1
>> BUG: spinlock recursion on CPU#0, pool_workqueue_/3
>> BUG: spinlock recursion on CPU#0, swapper/2
>> BUG: spinlock recursion on CPU#0, pool_workqueue_/3
>> BUG: spinlock recursion on CPU#0, pool_workqueue_/3
>> BUG: spinlock recursion on CPU#0, swapper/2
>> BUG: spinlock recursion on CPU#0, pool_workqueue_/3
>>
> Not the reliable reproducer I was hoping for but it is progress. We now
> know the problem shows up in both Aranym and Qemu.

Yep - no large impact by dropping the CPU clock (and BogoMIPS) by half 
though. One boot in sixteen shows the recursion message now.

Still running the test with local_irq_save() / local_irq_restore() 
protecting the spinlock debug tests.

>
>> Trying the same on a much faster Intel system, no messages are seen.
>> I'll try locking the PowerBook on half CPU clock rate next.
>>
>> ...
>>
>> The tests on unlocking certainly aren't atomic, but those are not the
>> ones we see in the messages. The test on locking use READ_ONCE() so
>> ought to be safe.
>>
>> The locking primitives are not atomic at all, by design ('No atomicity
>> anywhere, we are on UP'. While not debugging, spinlocks are NOPs on UP.)
>>
> I think spin_lock() reduces to preempt_disable() on UP.
> In include/linux/spinlock_api_up.h it says,
>
> /*
>   * In the UP-nondebug case there's no real locking going on, so the
>   * only thing we have to do is to keep the preempt counts and irq
>   * flags straight, to suppress compiler warnings of unused lock
>   * variables, and to add the proper checker annotations:
>   */

That's only true in the debug case - there, preempt_disable() is used 
inside the spin loop. But m68k is one of the last CONFIG_PREEMPT_NONE 
archs AFAIR, and preempt_disable() reduces to barrier() on those.

>> I wonder whether CONFIG_DEBUG_SPINLOCK was ever meant to work at all on UP?
>>
> I've no idea, sorry. The people who would be able to help would be found
> in MAINTAINERS in the "LOCKING PRIMITIVES" section.

I'll ask around once I've done a few more tests.

Cheers,

     Michael