Re: preemption and locking: why joined at the hip?

[Keir Fraser <keir.xen@gmail.com>]

On 31/08/2012 20:47, "Dan Magenheimer" <dan.magenheimer@oracle.com> wrote:

> Tracking down a tmem problem in 4.2.0-rcN that crashes the
> hypervisor, I've discovered a 4.2 changeset that forces
> a preemption_enable/disable for every lock/unlock.
> 
> Tmem has dynamically allocated "objects" that contain a
> lock.  The lock is held when the object is destroyed.
> No reason to unlock something that's about to be destroyed.
> But with the preempt_enable/disable in the generic locking code,
> and the fact that do_softirq ASSERTs that preempt_count
> must be zero, a crash occurs.
> 
> While I'm suitably embarrassed that tmem has not yet
> been tested with any recent -unstable, and I note that the
> workaround is simple (forcing an unlock before destroying the
> object containing the held lock), I have to ask if
> this change is really a good idea or is it unnecessary
> babysitting?

It is to prevent a vcpu from sleeping (eg on a waitqueue) while holding
spinlocks. If this were to happen, the possibility for deadlocks is obvious.
Hence it provides handy belt & braces sanity checking for this situation.

So just clean up after yourself and only destroy locks that are not locked.
;) I'm not clear why you'd be holding the lock during object destruction
anyway -- if anyone else could be spinning on the lock, it would not be safe
to free the lock.

 -- Keir

> Dan