From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:29552 "EHLO ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726828AbeJ0ORg (ORCPT ); Sat, 27 Oct 2018 10:17:36 -0400 Date: Sat, 27 Oct 2018 16:37:45 +1100 From: Dave Chinner To: Bart Van Assche Cc: Alexander Viro , linux-fsdevel@vger.kernel.org, Johannes Berg , Peter Zijlstra , Ingo Molnar , Theodore Ts'o Subject: Re: [PATCH RFC] kernel/locking, fs/direct-io: Introduce and use down_write_nolockdep() Message-ID: <20181027053745.GI6311@dastard> References: <20181026164905.214474-1-bvanassche@acm.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20181026164905.214474-1-bvanassche@acm.org> Sender: linux-fsdevel-owner@vger.kernel.org List-ID: On Fri, Oct 26, 2018 at 09:49:05AM -0700, Bart Van Assche wrote: > The rwsem locking functions contain lockdep annotations (down_write(), > up_write(), down_read(), up_read(), ...). Unfortunately lockdep and > semaphores are not a good match because lockdep assumes that releasing > a synchronization object will happen from the same kernel thread that > acquired the synchronization object. This is the case for most but not > all semaphore locking calls. When a semaphore is released from another > context than the context from which it has been acquired lockdep may > report a false positive circular locking report. This patch avoids > that lockdep reports the false positive report shown below for the > direct I/O code. Please note that the lockdep complaint shown below is > not the same as a closely related lockdep complaint that has been > reported recently by syzbot. This patch has been tested on top of a > patch that was suggested by Ted and Tejun, namely to change a > destroy_workqueue() call in the direct-io code into a call to > destroy_workqueue_no_drain(). For the syzbot report and the discussion > of that report, see also https://lkml.org/lkml/2018/10/23/511. FWIW, it kinda looks like you're recreating the rwsem debugging wrapper that's been in fs/xfs/mrlock.h since XFS was first ported to Linux. As an API, however, this needs to be consistent with down_read_non_owner()/up_read_non_owner() which are for exactly this same issue issue (different acquire/release contexts) on the read side of the rwsem. Indeed, it can probably use the same infrastructure... Cheers, Dave. -- Dave Chinner david@fromorbit.com