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 C8D3EEEB581 for ; Mon, 11 Sep 2023 00:55:55 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 057166B0157; Sun, 10 Sep 2023 20:55:55 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id F22326B0158; Sun, 10 Sep 2023 20:55:54 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D9C256B0159; Sun, 10 Sep 2023 20:55:54 -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 C34E06B0157 for ; Sun, 10 Sep 2023 20:55:54 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 8FBDA120751 for ; Mon, 11 Sep 2023 00:55:54 +0000 (UTC) X-FDA: 81222499428.12.8F6AF7C Received: from mail-pg1-f170.google.com (mail-pg1-f170.google.com [209.85.215.170]) by imf25.hostedemail.com (Postfix) with ESMTP id 9615FA0003 for ; Mon, 11 Sep 2023 00:55:52 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=fromorbit-com.20230601.gappssmtp.com header.s=20230601 header.b=NGlsSp6d; spf=pass (imf25.hostedemail.com: domain of david@fromorbit.com designates 209.85.215.170 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694393752; a=rsa-sha256; cv=none; b=mvhb0ckD+H9cJ1uNzHO8/Y+5WHTq795E75P8zwFMzuE18XqGUDoHiBS99SkyL16Z3+mMG6 qsynHv4MmPIatkSEd4jerQ4OsrbZHsBlHiYMRTwdyOzez5J0pPuUZaT5bMOr6Y83jVQL2G +3+4QbczGSwSYMfaHSJPRwbE3Ua4e6w= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=fromorbit-com.20230601.gappssmtp.com header.s=20230601 header.b=NGlsSp6d; spf=pass (imf25.hostedemail.com: domain of david@fromorbit.com designates 209.85.215.170 as permitted sender) smtp.mailfrom=david@fromorbit.com; dmarc=pass (policy=quarantine) header.from=fromorbit.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694393752; 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=gkCptR4EZl+Hose2vw9ASu1TJniSm3o/K3PjtvBEpBw=; b=fwGJ8IGmx5nrrSvUpVOod941JiMgIPuMsW+QWvAcaId2zr1i6RtZdDFtsAVWU1Hw4X1QCo JSAtCZ4zDGgdK0UcG4mImfc0o7a07+X/tJxUglOOeC/eMVTeV/psdu5yxOT4sdvIrhF+uW 9MHlBgoYcOKqE7aHegAXd4zDLnqtStk= Received: by mail-pg1-f170.google.com with SMTP id 41be03b00d2f7-56b2e689828so2914137a12.1 for ; Sun, 10 Sep 2023 17:55:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1694393751; x=1694998551; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=gkCptR4EZl+Hose2vw9ASu1TJniSm3o/K3PjtvBEpBw=; b=NGlsSp6dvOc06rwtza+4gou+SD8zjc4cpaP9OZhKiPBkkYEKDND6GLlD/67DwZEY46 uvG7Dt8yyQr5DJ6qFZlG0m2fVizxOCkEhDFVWGOA+QWeCMtb6mIYmqdzeP9DlRB7CzWS RkvwqZtop3UpU6PvGb9qcSbsRgHnOkVM8uQ9jAHRUMfIIHLWknKW/QSUt0ks64gEDTvH mQL0QHARhkEMyyRqjFbbFfD7iMLTWijtlxQbaXKVPxfFDGsob5EasY1fVg+Cki3WV6rR +6/skv0Hs/iVJASh6EMmDa1QIx8I+aQL0mjRWrlpsdowFyLvwpH9S1O1KJgJaF+UsBBo lxAw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694393751; x=1694998551; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=gkCptR4EZl+Hose2vw9ASu1TJniSm3o/K3PjtvBEpBw=; b=flmE473XnXVex59iTH0jsNc3A8lH41w669MIft9G631ud0pvbNjN+IzCenKnnimiqY xFWbcMVusgbyQaJ6y7dn1GHI0nenwc9xyZf5GcH9hYkDL4TtgIcvyWuocW++l2uzoq51 DmZHdsvxXTZNRpUghXvpUCEdz5YDbwt6tiVzAF3d0J2fYYwreN1SrlcdIWF52Pu3pC/+ lwMaaO8yV1iT9GcvDh9jEvxeHPzvdHtI1qcxbDlkgRdnr64fRpqRWUi4//7Yx/IC2Bi/ gQhf8eMEx/gXmyrcFBovFB3xdGn0StU6aNVrjSX825UX2zihlvR2bzSRUr2eBDSirWMf gCNQ== X-Gm-Message-State: AOJu0YztC7O9Ud1VdUgzBKmNiiHsdLsUDdUWcjHljuZ6eKE6yhSr1jJk ijy9WzqVsTzGwkCcYcep9SUMiQ== X-Google-Smtp-Source: AGHT+IH15x/YR43uoyl1Igs4BKI5BpHWhTAknaIVAizD2C3adaIwS+/qRVdEJvMrhTE7oaO6xaZlkA== X-Received: by 2002:a05:6a20:9489:b0:14c:512c:c138 with SMTP id hs9-20020a056a20948900b0014c512cc138mr8689849pzb.60.1694393751245; Sun, 10 Sep 2023 17:55:51 -0700 (PDT) Received: from dread.disaster.area (pa49-195-66-88.pa.nsw.optusnet.com.au. [49.195.66.88]) by smtp.gmail.com with ESMTPSA id x22-20020aa79196000000b0068bc014f352sm4447993pfa.7.2023.09.10.17.55.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 10 Sep 2023 17:55:50 -0700 (PDT) Received: from dave by dread.disaster.area with local (Exim 4.96) (envelope-from ) id 1qfVDQ-00DZQp-0P; Mon, 11 Sep 2023 10:55:48 +1000 Date: Mon, 11 Sep 2023 10:55:48 +1000 From: Dave Chinner To: Matthew Wilcox Cc: Peter Zijlstra , 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> <20230908104434.GB24372@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: 9615FA0003 X-Stat-Signature: x4zrqwwryq5ysnc61rcghtreagje8u4r X-Rspam-User: X-HE-Tag: 1694393752-489042 X-HE-Meta: U2FsdGVkX18zucjq15fE184sCg0nFp16NcB9POboxSlHheC9x7tR4njpILkF9KVaVOqOx25DQELT+V3faL4fCE1dBjtip+n/gpzE6DKV4n8e2RntxeV2Pf+CxB5Vq59VCJGAJLB/5jIRn7zyY9rGwqpp/Ia7JKtOF7I+Zl3sEohea9ius466/SohAPoBGtc6AIdf1iybL1FmVrfWCU29/t922aMf/rmhI7KXq0GM66TvWX6q5qQT8wXtxehdiRcj4oPM9kdH1PbOi+csiyc031QCngRVgJwLKdKJE8jRGD+UkUGYkPfsOSYZR8NuLfxnfOQOecko3C4cUFWEMiDCu8llAmt5JZNJMEKHezk41YzK6CkOIEdepjZ6Kes8T0MY0y9ByzFtNdglQsZ8D++1qlBJ640EAVm7rxE7UtgauteDQOFDOeH6AKcJt7VSIfCgOsSxxenz9wA6sA7WnRy4XD/o0OwKBiVgd4iUMsyKRcwcAPjBOMo9WE4eRiHdzUtWaYLd1BvHK2DIhZ1hNeMCYjeZR3VxmOwJXokH5A9PqCrOnFmNb63eTIYY8fzuh83teO2kUJipIm9JQTw04VnjEb4pw+PXB9ArdCPN3/cGmjpj/oVP3Dors99EqysYrxMDlnh5RKbMPTho8xXriqs/8hs4XdkKB1X1PHhkz2lfIBvLQFNvU2Ep52LowfV+3eRhuRRmvwldRkgmVuuWrxroRuW4VJqknEypfsX4ALA7raZk/fmyF8uNOea6oSo6Wpi8hD7cKq6RUZ6sZY+x7d57r7hioy01WQHjQA9nnWUQZvl7qTmbJnSaSvap6O4RefsiubIWS/5f+X8u7VzSDwIIhKNnuwvkrmg938AQzhcAGLsWmka2DhxjsU2an3F+0wkM4SfOuiPSai70tRErJzSfFeGRhoqSPpiyVg8QYab0PPyifzp577Zi5Bw7i8E50HYFtZpUqZ9orXxEzYxkgR8 tIHQ/uSt Lhrv1bT8JBdsS0Ak0Hbyt/BSwGpvIexKnRfVBUmokuh/hmC28jICu2aJ2hak7TF3HTnC48ezAgOk61WYXVJ7IcRBHtAgjuInzCSk9LtK3yBm9PC+mTCHwpCL+9Pb2Xpw+0ZsTJjmgxMXt4+KwItJXnWO+IoaBFbMko3sMdqlnrXYdY605cWK1CGfGApr/BsrfXpRiXUnc6nkzUYhgMimBpbw4p9sX9HXfLi3j7K6/THWKTwzRDz4HTRlDYJSjEM5Fe3p95aKqo6gUIrIZ9JzHIRWsDl51DTgTTNjGbtURR6BetWOdUgTMTvEDWmz3sWgxSkKAWDwH6KmnhnETZeeCN7BpbOl7N2MrJQ1/4oCBMEseb70hHYPs359PrWa8IsNo/HzSqtlfAONqLvLZGyZ//FroEMxLivjHZhqUUhPLWUHLT8pk+iZ/Diwq6F/9BZqbJSwHhwIgsjdj1Q6+gNHCGghLjc16yyNyM/JPxtLl3E2gD+AGFRhBsE0YnA== 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 Mon, Sep 11, 2023 at 12:17:18AM +0100, Matthew Wilcox wrote: > On Mon, Sep 11, 2023 at 08:56:45AM +1000, Dave Chinner wrote: > > On Fri, Sep 08, 2023 at 12:44:34PM +0200, Peter Zijlstra wrote: > > > 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. > > > > So your argument against such an interface is essentially "we can't > > have nice things because someone might abuse them"? > > Some people are very creative ... Sure, but that's no reason to stop anyone else from making progress. > I was thinking about how to handle this better. We could have > > static inline void rwsem_assert_locked(const struct rw_semaphore *sem) > { > BUG_ON(atomic_long_read(&sem->count) == 0); > } > > static inline void rwsem_assert_write_locked(const struct rw_semaphore *sem) > { > BUG_ON((atomic_long_read(&sem->count) & 1) != 1); > } We already have CONFIG_DEBUG_RWSEMS, so we can put these introspection interfaces inside debug code, and make any attempt to use them outside of debug builds break the build. e.g: #if DEBUG_RWSEMS /* * rwsem locked checks can only be used by conditionally compiled * subsystem debug code. It is not valid to use them in normal * production code. */ static inline bool rwsem_is_write_locked() { .... } static inline bool rwsem_is_locked() { .... } #else /* !DEBUG_RWSEMS */ #define rwsem_is_write_locked() BUILD_BUG() #define rwsem_is_locked() BUILD_BUG() #endif /* DEBUG_RWSEMS */ And now we simply add a single line to subsystem Kconfig debug options to turn on rwsem introspection for their debug checks like so: config XFS_DEBUG bool "XFS Debugging support" depends on XFS_FS + select RWSEM_DEBUG help Say Y here to get an XFS build with many debugging features, including ASSERT checks, function wrappers around macros, > but then we'd also need to change how XFS currently uses the ASSERT() > macro to be ASSERT_LOCKED(ip, flags), and I understand this code is also > used in userspace, so it'd involve changing that shim, and we're getting > way past the amount of code I'm comfortable changing, and way past the > amount of time I should be spending on this. > > And then there'd be the inevitable bikeshedding about "don't use BUG_ON" > and it's probably just for the best if I walk away at this point, > becoming the third person to fail to remove the mrlock. Yeah, which further points out how ridiculous the situation is. This is useful debug code and it can *obviously* and *easily* be constrained to debug environments. Cheers, Dave. -- Dave Chinner david@fromorbit.com