From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 41503EE57EF for ; Fri, 8 Sep 2023 10:44:43 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A2F726B00B0; Fri, 8 Sep 2023 06:44:42 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9DF376B00B1; Fri, 8 Sep 2023 06:44:42 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 8D3446B00B2; Fri, 8 Sep 2023 06:44:42 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 7AEDB6B00B0 for ; Fri, 8 Sep 2023 06:44:42 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 45143C1171 for ; Fri, 8 Sep 2023 10:44:42 +0000 (UTC) X-FDA: 81213096804.05.7B9422F Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf12.hostedemail.com (Postfix) with ESMTP id 5386D40003 for ; Fri, 8 Sep 2023 10:44:40 +0000 (UTC) Authentication-Results: imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KIE8pBuT; dmarc=none; spf=none (imf12.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=peterz@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694169880; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=CxXN7IaoL8RwVwQiUmo1siGJgoNZz5WFIpz2pAVAxuc=; b=XXbBOspvi+S6LuSGgdShylkv+a70ntt0en2jrDidlvt7WURkg2k/eOPSN00/9LY9iRyCvI VCNa9qBoSODiBFCgeLz0lSmaOcoWXzQR88iKxNLjQPcrPqG/hdaB/Y8FKygFmS4ETjCsDj CnUMH6X+Uhx9o3YvSQgyi1ht9C5XP4k= ARC-Authentication-Results: i=1; imf12.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=KIE8pBuT; dmarc=none; spf=none (imf12.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=peterz@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694169880; a=rsa-sha256; cv=none; b=Z7RTXLZfJyDidgTMyh91Mo9QLOLG/CRGbayXWjFKspcdIdoxavxZSdYo2M+RHDi3LstLcb pmITPYW5bl2gSCDKiLEAwj58ICJB+EBIZq4OH1Ya02Xd6MT8HozVM073c62JrhrVPc3fAb aAYQxH4Xuo9aeJlAsTSbGmPdag4u8NM= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=CxXN7IaoL8RwVwQiUmo1siGJgoNZz5WFIpz2pAVAxuc=; b=KIE8pBuT0ZFIv86asxj0niU4H1 3jPwIv8vg1avtC+nyDT0NIvw8I9ObyOPdsxNRjZ3vNAjDkNEptCLTUFypEo3+AFUy2bsdS0sUk0LI DrO4Icf0473OfWRp4uwkoWbT6Q9rbPJH8575k+dYGHfzAnkw1b6Li9kErMrBQBDyFs7PdHkxGxF+e cFawBf2j5TUjya/Y7hKCJKy77g/dikCTKSAgOlGqKH4hl1MSNVx/CEu/U/msdJNFtN9xpst2dlYwh FfqrbA3Z+sSVUsxSvR1rdDQtUYnjEgVtGl+qMoQ1re7dd6f4kOoypfVs15+QCyEYTUyDKEO0Gg4Sr 4Y0QLQwQ==; Received: from j130084.upc-j.chello.nl ([24.132.130.84] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.94.2 #2 (Red Hat Linux)) id 1qeYyZ-00HGsO-1U; Fri, 08 Sep 2023 10:44:35 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 9763C300472; Fri, 8 Sep 2023 12:44:34 +0200 (CEST) Date: Fri, 8 Sep 2023 12:44:34 +0200 From: Peter Zijlstra To: Dave Chinner Cc: Matthew Wilcox , Ingo Molnar , Will Deacon , Waiman Long , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Chandan Babu R , "Darrick J . Wong" , linux-xfs@vger.kernel.org Subject: Re: [PATCH 1/5] locking: Add rwsem_is_write_locked() Message-ID: <20230908104434.GB24372@noisy.programming.kicks-ass.net> References: <20230907174705.2976191-1-willy@infradead.org> <20230907174705.2976191-2-willy@infradead.org> <20230907190810.GA14243@noisy.programming.kicks-ass.net> <20230907193838.GB14243@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspam-User: X-Rspamd-Server: rspam12 X-Rspamd-Queue-Id: 5386D40003 X-Stat-Signature: iagwojs9xbfuisa5ofoxnbycd8745hx3 X-HE-Tag: 1694169880-48643 X-HE-Meta: U2FsdGVkX1+oFi3mSlzXuFcTWG/vpkpEQ49+9NFRwfWtB6FCXjK4O3nSa9fMPMIacTFFuyEGTG85xx+xxoyQknqQImT1R2fwteHt28t8w/jF7carWSpnBLyY7RtCqQtFatYJAwQnc+tSWEgkAxzkVxvET2/4BGcCCVTMYeRmiBJAUrF7zLuMIJ67zc0IHaVJJsG6rhXTLfzwfjD3yXmtOc4na+lvkHpFZeZM7rftQ9/Rq2XVfSdE9/UJ0+KIJwS6DZpYgAZm7VJJ4SAkoSexmyK408qZPyuf8rpfKgxKfE+u1hrfJfJ2EkkimE5ZszC52dk7d6Ai1EafFyxmk4c6UoHfaDCGjs6g4cqJCbegB7vrllECKX4Gum3xUo0gAFizlrWEH+SBTF09MLg3FMmUQiwfsqt/qJLg3mBjioyw/gdKCUIGe8KGfbjYUVlHmQQtXMjjnHuT196mJMjd65gGmNNulMco3IstdzINv39FconCiqyPRxYgHO8naGmZ52ufuXafrSduRzAx65XGSgFJZLVBwGPalgDY4/gp5cQ+EWz6xROQ0T5JmqI4WdgtBIa0QSxvl2azK2yrZSNrg5WNlRGh4e/kBE4LWOPJSXuIImT9H58HkGzzlYC/kZPeQVelIIF9r9FfjcRzVRUR4noE6/gI4oW1ARpOH7hON9o+FstIroRMzC/ga/Ew3xHo8zgG42L6W3rsBqBRFpBSgXtoZu/f9IDXekNgeGfNGtVQ4nNFJ8qLNEmBh9ToqcaIJ8ZMnf85njv7l8B1ivYGTFT7/y9cXYxEVx7jO8CV7b/I/8XQpSy4nrXEwcSRserawuaXzcYptGD5Fa2kwwdKXxLTszZTXG9Go7/fR845hooaKJHeR/CloZ89g4CWR92400Fnu7Jh8dfr9YuvmMkJsuEZ9wbG+GfGI9s5+a62DlauSC3VOo0dlVNqMFZhqZ6B/ug8Md5mQPIQSlxpuLypg0E w1dfkvkM SkaivYcLfOmdFzqZgbqgTzLZ6kv3yMIKFALXmMFlWDLE/oO/BuWgiraW0kUfRr8eFhyowt4JY5Z5o3oBNwk3G0BxtHyEKoqVMHq0978KCVn4xLEoQg9yTINzOQ4ZC850pjk/Rmsjq5WsdMPC1dnJVeE69s6FpWK7fiMyQOjh6g6ng9G9qqA5AjFn2xQ== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Sep 08, 2023 at 09:00:08AM +1000, Dave Chinner wrote: > > Right, but if you're not the lock owner, your answer to the question is > > a dice-roll, it might be locked, it might not be. > > Except that the person writing the code knows the call chain that > leads up to that code, and so they have a pretty good idea whether > the object should be locked or not. If we are running that code, and > the object is locked, then it's pretty much guaranteed that the > owner of the lock is code that executed the check, because otherwise > we have a *major lock implementation bug*. Agreed, and this is fine. However there's been some very creative 'use' of the _is_locked() class of functions in the past that did not follow 'common' sense. If all usage was: I should be holding this, lets check. I probably wouldn't have this bad feeling about things. > > Most devs should run with lockdep on when writing new code, and I know > > the sanitizer robots run with lockdep on. > > > > In general there seems to be a ton of lockdep on coverage. > > *cough* > > Bit locks, semaphores, and all sorts of other constructs for IO > serialisation (like inode_dio_wait()) have no lockdep coverage at > all. IOWs, large chunks of many filesystems, the VFS and the VM have > little to no lockdep coverage at all. True, however I was commenting on the assertion that vm code has duplicate asserts with the implication that was because not a lot of people run with lockdep on. > > > we also have VM_BUG_ON_MM(!rwsem_is_write_locked(&mm->mmap_lock), mm) > > > to give us a good assertion when lockdep is disabled. > > > > Is that really worth it still? I mean, much of these assertions pre-date > > lockdep. > > And we're trying to propagate them because lockdep isn't a viable > option for day to day testing of filesystems because of it's > overhead vs how infrequently it finds new problems. ... in XFS. Lockdep avoids a giant pile of broken from entering the kernel and the robots still report plenty. > > > XFS has a problem with using lockdep in general, which is that a worker > > > thread can be spawned and use the fact that the spawner is holding the > > > lock. There's no mechanism for the worker thread to ask "Does struct > > > task_struct *p hold the lock?". > > > > Will be somewhat tricky to make happen -- but might be doable. It is > > however an interface that is *very* hard to use correctly. Basically I > > think you want to also assert that your target task 'p' is blocked, > > right? > > > > That is: assert @p is blocked and holds @lock. > > That addresses the immediate symptom; it doesn't address the large > problem with lockdep and needing non-owner rwsem semantics. > > i.e. synchronous task based locking models don't work for > asynchronous multi-stage pipeline processing engines like XFS. The > lock protects the data object and follows the data object through > the processing pipeline, whilst the original submitter moves on to > the next operation to processes without blocking. > > This is the non-blocking, async processing model that io_uring > development is pushing filesystems towards, so assuming that we only > hand a lock to a single worker task and then wait for it complete > (i.e. synchronous operation) flies in the face of current > development directions... I was looking at things from an interface abuse perspective. How easy is it to do the wrong thing. As said, we've had a bunch of really dodgy code with the _is_locked class of functions, hence my desire to find something else. As to the whole non-owner locking, yes, that's problematic. I'm not convinced async operations require non-owner locking, at the same time I do see that IO completions pose a challence. Coming from the schedulability and real-time corner, non-owner locks are a nightmare because of the inversions. So yeah, fun to be had I'm sure.