From mboxrd@z Thu Jan 1 00:00:00 1970 From: Keir Fraser Subject: Re: preemption and locking: why joined at the hip? Date: Fri, 31 Aug 2012 21:08:02 +0100 Message-ID: References: <16d6e254-acff-4212-b5ed-0b3dd5b40ea5@default> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <16d6e254-acff-4212-b5ed-0b3dd5b40ea5@default> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Dan Magenheimer , xen-devel@lists.xen.org Cc: Zhenzhong Duan , Konrad Rzeszutek Wilk List-Id: xen-devel@lists.xenproject.org On 31/08/2012 20:47, "Dan Magenheimer" 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