Re: [PATCH v3 1/1] ppc/crash: Reset spinlocks during crash

[Peter Zijlstra]

On Tue, Mar 31, 2020 at 09:00:21PM -0300, Leonardo Bras wrote:
> During a crash, there is chance that the cpus that handle the NMI IPI
> are holding a spin_lock. If this spin_lock is needed by crashing_cpu it
> will cause a deadlock. (rtas.lock and printk logbuf_lock as of today)
> 
> This is a problem if the system has kdump set up, given if it crashes
> for any reason kdump may not be saved for crash analysis.
> 
> After NMI IPI is sent to all other cpus, force unlock all spinlocks
> needed for finishing crash routine.
> 
> Signed-off-by: Leonardo Bras <leonardo@xxxxxxxxxxxxx>

> @@ -129,6 +132,13 @@ static void crash_kexec_prepare_cpus(int cpu)
>  	/* Would it be better to replace the trap vector here? */
>  
>  	if (atomic_read(&cpus_in_crash) >= ncpus) {
> +		/*
> +		 * At this point no other CPU is running, and some of them may
> +		 * have been interrupted while holding one of the locks needed
> +		 * to complete crashing. Free them so there is no deadlock.
> +		 */
> +		arch_spin_unlock(&logbuf_lock.raw_lock);
> +		arch_spin_unlock(&rtas.lock);
>  		printk(KERN_EMERG "IPI complete\n");
>  		return;
>  	}

You might want to add a note to your asm/spinlock.h that you rely on
spin_unlock() unconditionally clearing a lock.

This isn't naturally true for all lock implementations. Consider ticket
locks, doing a surplus unlock will wreck your lock state in that case.
So anybody poking at the powerpc spinlock implementation had better know
you rely on this.