Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753641AbdHQUbC (ORCPT ); Thu, 17 Aug 2017 16:31:02 -0400 Received: from mail.kernel.org ([198.145.29.99]:45010 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753586AbdHQUbA (ORCPT ); Thu, 17 Aug 2017 16:31:00 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6CD5922B4E Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Thu, 17 Aug 2017 16:30:56 -0400 From: Steven Rostedt To: Waiman Long Cc: Jens Axboe , Jeff Layton , "J. Bruce Fields" , Ingo Molnar , linux-kernel@vger.kernel.org, linux-block@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v2] blktrace: Fix potentail deadlock between delete & sysfs ops Message-ID: <20170817163056.19ea42c8@gandalf.local.home> In-Reply-To: References: <1502916040-18067-1-git-send-email-longman@redhat.com> <20170817093444.3276f7ab@gandalf.local.home> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2341 Lines: 61 On Thu, 17 Aug 2017 12:24:39 -0400 Waiman Long wrote: > On 08/17/2017 09:34 AM, Steven Rostedt wrote: > > On Wed, 16 Aug 2017 16:40:40 -0400 > > Waiman Long wrote: > > > >> The lockdep code had reported the following unsafe locking scenario: > >> > >> CPU0 CPU1 > >> ---- ---- > >> lock(s_active#228); > >> lock(&bdev->bd_mutex/1); > >> lock(s_active#228); > >> lock(&bdev->bd_mutex); > > Can you show the exact locations of these locks. I have no idea where > > this "s_active" is. > The s_active isn't an actual lock. It is a reference count (kn->count) > on the sysfs (kernfs) file. Removal of a sysfs file, however, require > a wait until all the references are gone. The reference count is > treated like a rwsem using lockdep instrumentation code. Which kernel is this? I don't see any lockdep annotation around kn->count (doing a git grep, I find it referenced in fs/kernfs/dir.c) > > >> *** DEADLOCK *** > >> > >> The deadlock may happen when one task (CPU1) is trying to delete > >> a partition in a block device and another task (CPU0) is accessing > >> tracing sysfs file in that partition. > >> > >> To avoid that, accessing tracing sysfs file will now use a mutex > >> trylock loop and the operation will fail if a delete operation is > >> in progress. > >> > >> Signed-off-by: Waiman Long > >> --- > >> > >> v2: > >> - Use READ_ONCE() and smp_store_mb() to read and write bd_deleting. > >> - Check for signal in the mutex_trylock loops. > >> - Use usleep() instead of schedule() for RT tasks. > > I'm sorry but I really do hate this patch. > > Any suggestion on how to make it better? I'd like to be able to at least trigger the warning. And see the lock issues. I wont be able to recommend anything until I understand what is happening. > The root cause is the lock inversion under this circumstance. I think > modifying the blk_trace code has the least impact overall. I agree that > the code is ugly. If you have a better suggestion, I will certainly like > to hear it. Again, I need to see where the issue lies before recommending something else. I would hope there is a more elegant solution to this. -- Steve