Re: [PATCH] x86/spinlocks: Avoid a deadlock when someone unlock a zapped ticked spinlock

[Petr Mladek]

On Wed 2015-10-21 13:11:20, Peter Zijlstra wrote:
> On Wed, Oct 21, 2015 at 11:18:09AM +0200, Petr Mladek wrote:
> > There are few situations when we reinitialize (zap) ticket spinlocks. It
> > typically happens when the system is going down after an error and we
> > want to avoid deadlock in some important services. For example,
> > zap_locks() in printk.c and ioapic_zap_locks().
> 
> So there's a few problems here. On x86 the code you patch is dead code,
> x86 no longer uses ticket locks. Other archs might still.
>
> And I entirely detest adding instructions to any lock path, be it the
> utmost fast path or not, for something that will _never_ happen (on a
> healthy system).

OK

> I would still very much recommend getting rid of the need for
> zap_locks() in the first place.
>
> What I did back when is punt on the whole printk buffer madness and dump
> things to early_printk() without any locking.

My problem with this approach is that the early console is a black hole if
people do not watch it on a remote machine. We would stop using the
really used console even when it would work in most cases. IMHO,
zap_locks() is far from ideal but it would give better results in most
cases.

Maybe, we could add support to use early console in case of Oops and
panic. But we should not do this by default. It would help, especially
when the problem is reproducible and we get stacked with the normal
console.


> I think that as long as the printk buffer has locks you have to accept
> to loose some data when really bad stuff goes down.

Yup, but we could try a bit harder. I am afraid that lockless buffer,
e.g. ring_buffer is not an option here because it would be too complex,
rather slow, and hard to handle in a crash dump.

Thanks a lot for feedback.


Best Regards,
Petr
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/