From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1161156AbcBQKk5 (ORCPT ); Wed, 17 Feb 2016 05:40:57 -0500 Received: from casper.infradead.org ([85.118.1.10]:50479 "EHLO casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030266AbcBQKkz (ORCPT ); Wed, 17 Feb 2016 05:40:55 -0500 Date: Wed, 17 Feb 2016 11:40:49 +0100 From: Peter Zijlstra To: Byungchul Park Cc: willy@linux.intel.com, akpm@linux-foundation.org, mingo@kernel.org, linux-kernel@vger.kernel.org, akinobu.mita@gmail.com, jack@suse.cz, sergey.senozhatsky.work@gmail.com, peter@hurleysoftware.com Subject: Re: [PATCH v3] lock/semaphore: Avoid an unnecessary deadlock within up() Message-ID: <20160217104049.GB25010@twins.programming.kicks-ass.net> References: <1455700311-2308-1-git-send-email-byungchul.park@lge.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1455700311-2308-1-git-send-email-byungchul.park@lge.com> User-Agent: Mutt/1.5.21 (2012-12-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 17, 2016 at 06:11:51PM +0900, Byungchul Park wrote: > change from v2 to v3 > - the way to solve it in v2 is racy, so I changed the approach entirely. > - just make semaphore's trylock use spinlock's trylock. > > change from v1 to v2 > - remove unnecessary overhead by the redundant spin(un)lock. > > Since I faced a infinite recursive printk() bug, I've tried to propose > patches the title of which is "lib/spinlock_debug.c: prevent a recursive > cycle in the debug code". But I noticed the root problem cannot be fixed > by that, through some discussion thanks to Sergey and Peter. So I focused > on preventing the deadlock. > > -----8<----- > From 43e029ca920890ac644e30d873be69cf5d01efdb Mon Sep 17 00:00:00 2001 > From: Byungchul Park > Date: Wed, 17 Feb 2016 17:22:18 +0900 > Subject: [PATCH v3] lock/semaphore: Avoid an unnecessary deadlock within up() > > One of semaphore acquisition functions, down_trylock() is implemented > using raw_spin_lock_irqsave(&sem->lock) even though it's enough to use > raw_spin_trylock_irqsave(). Furthermore, using raw_spin_lock_irqsave() > can cause a unnecessary deadlock as described below. Just make it use > the spinlock trylock to implement the semaphore trylock so that we can > avoid the unnecessary deadlock happened. > > The scenario the bad thing can happen is, > > printk > console_trylock > console_unlock > up_console_sem > up > raw_spin_lock_irqsave(&sem->lock, flags) > __up > wake_up_process > try_to_wake_up > raw_spin_lock_irqsave(&p->pi_lock) > __spin_lock_debug > spin_dump > printk > console_trylock > raw_spin_lock_irqsave(&sem->lock, flags) > >>> DEADLOCK <<< > > Signed-off-by: Byungchul Park Mucking with the semaphore implementation just because printk() is terminally broken shite really doesn't fly.