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 6B63FEC875F for ; Fri, 8 Sep 2023 00:01:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A1DD08D0002; Thu, 7 Sep 2023 20:01:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9CDED6B007D; Thu, 7 Sep 2023 20:01:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 895CF8D0002; Thu, 7 Sep 2023 20:01:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 796866B007B for ; Thu, 7 Sep 2023 20:01:18 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 3B2F640E09 for ; Fri, 8 Sep 2023 00:01:18 +0000 (UTC) X-FDA: 81211475436.11.6C8672E Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf27.hostedemail.com (Postfix) with ESMTP id 45A9340012 for ; Fri, 8 Sep 2023 00:01:15 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=ODLmaq3b; dmarc=none; spf=none (imf27.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694131275; 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=kFxMwjKPpQPRz2kPrpu3oF5FvWBvyL3/WDSSiKgMZu0=; b=AYI79tevC9yDOd+bRZI8dv4hL/fVB0wrEvFlvIVokR8SCxtx9EMteNZmrdXGhm8SB62/UI ijqaHDIAkGqeb3Bo5p86Y9f27fzyDVid260FxX6CSV6waluVpST2bqfFInGwnGD34sbRzG 3h50aWHUwGv+8mzbhyuob3iiv7Llfsc= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=ODLmaq3b; dmarc=none; spf=none (imf27.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694131275; a=rsa-sha256; cv=none; b=ZAIVMh4QgCY1A02OPEymXG2rqKoi56rpdaI8rJ/rcKtOAp4yOygPlI2eZ0cx6L3huN5M/C S0cxmoMaok8ugyfx2cuBMldqFwOqsU7KFkC1NhosUwyIqX6Rd0uL1v2jZLLi9eRwiM60LN NS08XF8uk6TVN1EQ8avHw/kO5klL/dk= 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=kFxMwjKPpQPRz2kPrpu3oF5FvWBvyL3/WDSSiKgMZu0=; b=ODLmaq3brgjdrxOtQK5P2QaqtN uMwWXT4Y5V4aY9d5N/hH7PDlzh8ijv8er3X1mvjbcNskxEvs05ZDQUZvck2X5PlaND+ymUVfrRmuq KdSN3Ohl8MmUTXPy6fyL/vUs+C9X96otk+Bxsh8/PL4mhszDkeXFHnX85wzNPvK+bFNvQuCjZTGD7 Y3q0oH4fFYB1P7yaAuaGZy+hCBPQd9N1QOcxEdBjvfo1BSu/MeTiqOwUC0yqga8aFpV61dCvDahxr MXbiqbPeyHG08SaqvmEkdTSpy/sio7q6dGjoHuUOY2bxmp4DoKRvMBQrcZlNduqn0Zse2XdAgvLJv Wx1x4dKw==; Received: from willy by casper.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1qeOvn-00EJfd-Mq; Fri, 08 Sep 2023 00:01:03 +0000 Date: Fri, 8 Sep 2023 01:01:03 +0100 From: Matthew Wilcox To: Peter Zijlstra Cc: 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: 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: <20230907193838.GB14243@noisy.programming.kicks-ass.net> X-Rspamd-Queue-Id: 45A9340012 X-Rspam-User: X-Rspamd-Server: rspam04 X-Stat-Signature: 8a6gr8t9it7g3spxwonwi9mfu985mxj3 X-HE-Tag: 1694131275-650951 X-HE-Meta: U2FsdGVkX18noPEQ0po0qATa/CZpS8o7DNTc1WXylynGt7GPjDdH0FmGA4/atusIcVPCmEb8VAMnj7ip4+ZU337Hocgm+Vbfg5PHRXL5/ymEiue7oC3MVZOLR8ytXo1A2fkCFj7ZqHNs0m9COTfhPTrcn55+UY8mw2DWTH4TqCYDwH3mWLpCkQ3PEDwaLH8KEVQkn4OkEZbanR4Oxgq/gC9M4jwyk2iP4q5Lt26YKwxLjVlkj6vR8UCaanoSdZvLnkW0f2HKUDHkD7GXxuJGxSOXTVQnSUq+Nvz5RNaRHhMNoARdm285f9+qHidE6w2Wne9NoKU7O84C+rRiaO1TGWK2ajpzYeyutMAxFilhVCzYjlaz4K7F9y5voaw/W0GRbQOfV1eWPNdUARm2mFCFxnUSIhBIP7eGbhXimASLIUytQrQ9jFGAfykboncapT7iJqCz9wJDaQF2jd19agmJW7aX22Po+5u6bxqwlet4uaWjzEraycudHsAw5LcenB9YeYtGS6u3IFbGvK7JjAXEi0CrHIbB1Lc1v80eIi4TQ++CI56Ls2nGPlnwlW+DCDmsaTLrrNkvQlnd7BIhFydT9CARfhcf55iuK695Vwqx37AZ0wky5L/lOXI409qo+ZPQdop1bXgPzOrz/KOB0GMz8xcduCpLBf6v7lOdQ1cL6EhzCsftEBxl9KeKgYxqA0RkYEAW2dyVLCXBhmE6iKV3yXZKHWuJWbN2KMjppTEaEzFPKmxRatz7B/BPVfGODDbrglT6owHP5YBnfbxRBk46EZ3h4wxY6lGQjyRZnbTOfgS0s+0KzBe0iXAlwcwMAChiWQqdnuX/pbCk1Z4FvooZZRL+TS7hbAJg1aKy2ZcVyTzOgrSVjk5L/BcvztLvqarqUzLN0eA+pT2z5ftghoJm5l510ZqDr8V2j80xXj2DDRJ+9qeJzKTdJSCWb3zq8XWGZSh6LKEKvivWv7Gxjq1 Cvu5yPId JxXp2+9uTLk4XTJkjRg4LnlWLOYzyJPutVXo6fCpxCb5yQmQQJu36k3dGLHqVaqKgPD2Kl0/AZ78SkL1ylZn8fR81ef9PZ4aCaOmI8fmiFcnqnb/MfkC+TGxmls9STP0H0cygFaROOuulbdjF+QmZjTuFRafGV9Nn0y9+Uqv490sSKFhaUbIRJUa5og== 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 Thu, Sep 07, 2023 at 09:38:38PM +0200, Peter Zijlstra wrote: > On Thu, Sep 07, 2023 at 08:20:30PM +0100, Matthew Wilcox wrote: > > On Thu, Sep 07, 2023 at 09:08:10PM +0200, Peter Zijlstra wrote: > > > On Thu, Sep 07, 2023 at 06:47:01PM +0100, Matthew Wilcox (Oracle) wrote: > > > > Several places want to know whether the lock is held by a writer, instead > > > > of just whether it's held. We can implement this for both normal and > > > > rt rwsems. RWSEM_WRITER_LOCKED is declared in rwsem.c and exposing > > > > it outside that file might tempt other people to use it, so just use > > > > a comment to note that's what the 1 means, and help anybody find it if > > > > they're looking to change the implementation. > > > > > > I'm presuming this is deep in a callchain where they know they hold the > > > lock, but they lost in what capacity? > > > > No, it's just assertions. You can see that in patch 3 where it's > > used in functions called things like "xfs_islocked". > > 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. Sure, but if you are the lock owner, it's guaranteed to work. And if you hold it for read, and check that it's held for write, that will definitely fail. I agree "somebody else is holding it" gives you a false negative, but most locks aren't contended, so it'll fail the assertion often enough. > > Patch 2 shows it in use in the MM code. We already have a > > lockdep_assert_held_write(), but most people don't enable lockdep, so > > 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. Hm. Enabling lockdep causes an xfstests run to go up from 6000 seconds to 8000 seconds for me. That's a significant reduction in the number of test cycles I can run per day. > > 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. That's true. Is it possible to do a cheaper version of lockdep where it only records that you have the lock and doesn't, eg, check for locking dependencies? I haven't analysed lockdep to see what the expensive part is; for all I know it's recording the holders, and this idea is worthless. > > 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. Probably, although I think there might be a race where the worker thread starts running before the waiter starts waiting? In conversation with Darrick & Dave, XFS does this in order to avoid overflowing the kernel stack. So if you've been thinking about "we could support segmented stacks", it would avoid having to support this lockdep handoff. It'd probably work a lot better for the scheduler too.