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 18B8EC71153 for ; Mon, 11 Sep 2023 02:16:08 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 4B4A46B014E; Sun, 10 Sep 2023 22:16:07 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 43E086B0154; Sun, 10 Sep 2023 22:16:07 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 2DE9D6B015B; Sun, 10 Sep 2023 22:16:07 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 1A1C86B014E for ; Sun, 10 Sep 2023 22:16:07 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D3F22407C0 for ; Mon, 11 Sep 2023 02:16:06 +0000 (UTC) X-FDA: 81222701532.18.8E7BEA5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf25.hostedemail.com (Postfix) with ESMTP id F090AA0008 for ; Mon, 11 Sep 2023 02:16:04 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=f1e7cnDa; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf25.hostedemail.com: domain of longman@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=longman@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1694398565; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=S/3lrXi36lgJXFC9YTvAy+juUVj3+3Iz/BRPulL9kwg=; b=r8RCm4FWwaZXCDJWLtGEZ5H2YSI8rcMszvb7SHWVs2J+0MpItyv4PH86mKitJWz7vTFJ3h c18TZR3PyvvIGHVMf73lVq5PFs6+hT0LvKAGYePPh+lnSt5A5TbINoRVpbh1Uvxf+OgkkX +Czg0kfJaH5bFUSIbrvjBtbXR9UvXc4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=f1e7cnDa; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf25.hostedemail.com: domain of longman@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=longman@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1694398565; a=rsa-sha256; cv=none; b=X7ccpVuTaLjdRbAYHh4PB75yUv+otXES9F7j5cY2nObd60naE/0AYVWn+5FK78I3Z0OF2H 8/BwkFB85AFw+ZN3VJXsI/dZI6LBe/iGfU1CUJPEfAbbpSvHBrcLmU5sQ1wYd4YT+LPXi6 9kmN9P0wswqLZD5V6da99tp3zCptiqk= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1694398564; h=from:from: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:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=S/3lrXi36lgJXFC9YTvAy+juUVj3+3Iz/BRPulL9kwg=; b=f1e7cnDajWwvspj8hHLucTNWweqglHqG0+q85e79PxcS9O2S9rlXwFcb184LzgQpO+q0jE 83oLk5QPGhRxmxt5JzDlGVEE5mFYbwGjmSIDrN6QSOndwPE8jaOpCaV15W/lT9TO6wuruK BENOsuj8wEYavmB6xiUNJXYFC7ru6RA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-592-4YI_k2z9PEK9m95hQDnNUQ-1; Sun, 10 Sep 2023 22:16:00 -0400 X-MC-Unique: 4YI_k2z9PEK9m95hQDnNUQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.rdu2.redhat.com [10.11.54.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E4F00816502; Mon, 11 Sep 2023 02:15:59 +0000 (UTC) Received: from [10.22.8.52] (unknown [10.22.8.52]) by smtp.corp.redhat.com (Postfix) with ESMTP id 70D69200B401; Mon, 11 Sep 2023 02:15:59 +0000 (UTC) Message-ID: <70d89bf4-708b-f131-f90e-5250b6804d48@redhat.com> Date: Sun, 10 Sep 2023 22:15:59 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.14.0 Subject: Re: [PATCH 1/5] locking: Add rwsem_is_write_locked() Content-Language: en-US To: Dave Chinner , Matthew Wilcox Cc: Peter Zijlstra , Ingo Molnar , Will Deacon , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Chandan Babu R , "Darrick J . Wong" , linux-xfs@vger.kernel.org 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> From: Waiman Long In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 3.1 on 10.11.54.4 X-Rspam-User: X-Stat-Signature: 5ejaia8om9ohy1n75cums64c4eerc63b X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: F090AA0008 X-HE-Tag: 1694398564-810116 X-HE-Meta: U2FsdGVkX1917q3ijMxgN8aFosk4Tf8e0qfQKlZE1OvQ2l9vXSfJJ3DfPPRPbwUthqAN4GzJadJZQdPmVto9fQyY9RLqL9HHFDqDDXmL4f3mH2uysm/6ECP5BwaxyRYj+ZoUtsXZn7vP2yL3WX2vAPVIUgIMn/LZRmZr24vzEeeyyDNgtgqr2kGOJkColj23+Jvfm9nxf5MvR8m+jjXIcjLgx4Ti2/sXmtTV9r7wOYhHWQoz+vARk0RN0RK/SYJHxmkHuY3tXdyl8H57MEwDlIJfsLXpW70Siyv1UHXbzbMDsh0smS6vAVHlMCf2EPfKcM6ffJwGpk3NmRY5CSf0t41vzCn+VXT5zd33IFCWnv67MntDtkoTPvFGE7KEjkNXNCGt++q/sFFlOp48N9m6Dj9gCrTcUnhtvQtkgy1Dx5rpdjG66v/A/QUsNpgwMR1iu1lWjiQN5HsCPliGj0JtablUO+azskN6GAY781EY67/pbvk88nrlPFfE2SyOwnNHfH62mvhZyeb4qnzWTx/2LR32FDogS2UejFTlABZfaUmrhdreRDC5mCQc0Y9JKoJzd2NUcsxgysz6hinuw1PIhLdf+ggM5tygBKOOOjuyhpiUmwhYi5Yr3LKnfpdjjZ0qfvoX8oXSbfEIMRsQO0VqF+svETHNJQXM8LG05mQpBI+1KmqvCUwNmq6DHPSTVMIf0rZDZJKu7nI8owWYDK4IzMCSkdhL5jBxKnY9rYCYx0G80m4lFJoI61FQ3iYT7XoczSXqrevsv5AEqh/kWVVbBjkHv6EojZ9vjljaw1nFOWBOgd0CFQrKyu0saFV2w0N2nnUkKDZOK0a1eust/eCP0emlz+4NXu5bDv1RchfZaFJyQREp8+ipub3Jdx1kvL+yLH+L7bTAwUMP+mnKQFuNLmScLc0VsmaU9aHaB4lT/dyBhYNk2oEnMTf4gRf6WG2RZBD2lDsai5bSljdezna 5eriRHg2 PMrHcJLWEGuXQUrvnu+Y0980Bojh7ursTuEvq1gHf7mM71sHJ/giUVxIh9RgtoshI3BkS0EyGYAZx7SRdf+u7YBgbOXIwvOeGX77IM4guoWA4QRLBDyMH5YCjnJNmM/DevHaeOqjCT/Rv58e/xOIjfh+pUasI8wSgcvU4DGBFUesJC7uUVOhQCeHWc/duzfJhuZMhpMxSE02wBhuM0t1rL9XeKA== 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 9/10/23 20:55, Dave Chinner wrote: > 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, That may be a possible compromise. Actually, I am not against having them defined even outside the DEBUG_RWSEMS. We already have mutex_is_locked() defined and used in a lot of places. So this is just providing the rwsem equivalents. I also agreed that these APIs can be misused by other users. I think we should clearly document the caveats of using these. So unless there are other means of maintaining the stability of the lock state, the test result may no longer be true right after the test. It is simply just the lock state at a certain moment in time. Callers are using them at their own risk. Cheers, Longman