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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 83909EE14C3 for ; Wed, 6 Sep 2023 18:43:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242082AbjIFSnz (ORCPT ); Wed, 6 Sep 2023 14:43:55 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53722 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234158AbjIFSnu (ORCPT ); Wed, 6 Sep 2023 14:43:50 -0400 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 858B88E; Wed, 6 Sep 2023 11:43:37 -0700 (PDT) Received: by smtp.kernel.org (Postfix) with ESMTPSA id DF48AC433C8; Wed, 6 Sep 2023 18:43:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1694025817; bh=4LpTj4dO1DZp+wPt1j326b9RHl688VNjoFRDBK7yCjo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=rMYo4nD1ZhlGaViMjhHsNMan5ePfVetvQknQBxgotWKktnC6a6kvmyycmIb1nX0nt 1HTNSqKl2CAWzyv6tMz/5xA2IuIphy9Rs4r1jICSpcMmkJrqpLlVjS/j22/39fIvjE A8jx840OcGXTtxvPAKq5H5X6V01XRgO71UX6vMi6YimjX0Hn6+ZeSONiir3/cIyCZx fJgZQ201+Kz22p188mf+ZUuZOraS8DC3ZgnX7dgc0eQ/gzy308Hv9AwuuzPgMPou2X GhCttd85s4lUbsuP05wouMWyhd2PJAasHNi2jmVapCwDxBJzAPcu7QDH9r6K2QeYQW NgLGEXg6M5nrg== Date: Wed, 6 Sep 2023 11:43:36 -0700 From: "Darrick J. Wong" To: Matthew Wilcox Cc: Bernd Schubert , Mateusz Guzik , brauner@kernel.org, viro@zeniv.linux.org.uk, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [RFC PATCH] vfs: add inode lockdep assertions Message-ID: <20230906184336.GH28160@frogsfrogsfrogs> References: <20230831151414.2714750-1-mjguzik@gmail.com> <20230906152948.GE28160@frogsfrogsfrogs> <20230906170724.GI28202@frogsfrogsfrogs> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org On Wed, Sep 06, 2023 at 07:33:48PM +0100, Matthew Wilcox wrote: > On Wed, Sep 06, 2023 at 10:07:24AM -0700, Darrick J. Wong wrote: > > On Wed, Sep 06, 2023 at 05:00:14PM +0100, Matthew Wilcox wrote: > > > +++ b/fs/xfs/xfs_inode.c > > > @@ -361,7 +361,7 @@ xfs_isilocked( > > > { > > > if (lock_flags & (XFS_ILOCK_EXCL|XFS_ILOCK_SHARED)) { > > > if (!(lock_flags & XFS_ILOCK_SHARED)) > > > - return !!ip->i_lock.mr_writer; > > > + return rwsem_is_write_locked(&ip->i_lock.mr_lock); > > > > You'd be better off converting this to: > > > > return __xfs_rwsem_islocked(&ip->i_lock.mr_lock, > > (lock_flags & XFS_ILOCK_SHARED)); > > > > And then fixing __xfs_rwsem_islocked to do: > > > > static inline bool > > __xfs_rwsem_islocked( > > struct rw_semaphore *rwsem, > > bool shared) > > { > > if (!debug_locks) { > > if (!shared) > > return rwsem_is_write_locked(rwsem); > > return rwsem_is_locked(rwsem); > > } > > > > ... > > } > > Thanks. > > > > +++ b/include/linux/rwsem.h > > > @@ -72,6 +72,11 @@ static inline int rwsem_is_locked(struct rw_semaphore *sem) > > > return atomic_long_read(&sem->count) != 0; > > > } > > > > > > +static inline int rwsem_is_write_locked(struct rw_semaphore *sem) > > > +{ > > > + return atomic_long_read(&sem->count) & 1; > > > > > > atomic_long_read(&sem->count) & RWSEM_WRITER_LOCKED ? > > Then this would either have to be in rwsem.c or we'd have to move the > definition of RWSEM_WRITER_LOCKED to rwsem.h. All three options are > kind of bad. I think I hate the bare '1' least. I disagree, because using the bare 1 brings the most risk that someone will subtly break the locking assertions some day when they get the bright idea to move RWSEM_WRITER_LOCKED to the upper bit and fail to notice this predicate and its magic number. IMO moving it to the header file (possibly with the usual __ prefix) would be preferable to leaving a landmine. --D