From: Waiman Long <longman@redhat.com>
To: Matthew Wilcox <willy@infradead.org>
Cc: Peter Zijlstra <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>, Will Deacon <will@kernel.org>,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
Chandan Babu R <chandan.babu@oracle.com>,
"Darrick J . Wong" <djwong@kernel.org>,
linux-xfs@vger.kernel.org, Mateusz Guzik <mjguzik@gmail.com>
Subject: Re: [PATCH v3 1/4] locking: Add rwsem_assert_held() and rwsem_assert_held_write()
Date: Thu, 16 Nov 2023 20:50:45 -0500 [thread overview]
Message-ID: <767ce2df-6f9b-40a3-b40b-e9e7a593e3b2@redhat.com> (raw)
In-Reply-To: <ZVY/cAMFbkuKJF/Y@casper.infradead.org>
On 11/16/23 11:12, Matthew Wilcox wrote:
> On Tue, Nov 14, 2023 at 08:17:32PM -0500, Waiman Long wrote:
>>>> There are some inconsistency in the use of WARN_ON() and BUG_ON() in the
>>>> assertions. For PREEMPT_RT, held_write is a BUG_ON. For non-PREEMPT_RT, held
>>>> is a BUG_ON. It is not clear why one is BUG_ON and other one is WARN_ON. Is
>>>> there a rationale for that?
>>> I'll fix that up.
>> The check for write lock ownership is accurate. OTOH, the locked check can
>> have false positive and so is less reliable.
> When you say 'false positive', do you mean it might report the lock as
> being held, when it actually isn't, or report the lock as not being held
> when it actually is? The differing polarities of assert and BUG_ON
> make this confusing as usual.
It means there may be no active lock owner even though the count isn't
zero. If there is one or more owners, the count will always be non-zero.
>
> Obviously, for an assert, we're OK with it reporting that the lock is
> held when actually it's not. The caller is expected to hold the lock,
> so failing to trip the assert when the caller doesn't hold the lock
> isn't great, but we can live with it. OTOH, if the assert fires when
> the caller does hold the lock, that is not tolerable.
The second case shouldn't happen. So the assert should be OK.
Cheers,
Longman
next prev parent reply other threads:[~2023-11-17 1:50 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-10 20:41 [PATCH v3 0/4] Remove the XFS mrlock Matthew Wilcox (Oracle)
2023-11-10 20:41 ` [PATCH v3 1/4] locking: Add rwsem_assert_held() and rwsem_assert_held_write() Matthew Wilcox (Oracle)
2023-11-10 22:21 ` Waiman Long
2023-11-14 21:26 ` Matthew Wilcox
2023-11-15 1:17 ` Waiman Long
2023-11-16 16:12 ` Matthew Wilcox
2023-11-17 1:50 ` Waiman Long [this message]
2023-11-13 8:24 ` Peter Zijlstra
2023-11-10 20:41 ` [PATCH v3 2/4] mm: Use rwsem assertion macros for mmap_lock Matthew Wilcox (Oracle)
2023-11-10 20:41 ` [PATCH v3 3/4] xfs: Replace xfs_isilocked with xfs_assert_ilocked Matthew Wilcox (Oracle)
2023-11-10 20:41 ` [PATCH v3 4/4] xfs: Remove mrlock wrapper Matthew Wilcox (Oracle)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=767ce2df-6f9b-40a3-b40b-e9e7a593e3b2@redhat.com \
--to=longman@redhat.com \
--cc=chandan.babu@oracle.com \
--cc=djwong@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mjguzik@gmail.com \
--cc=peterz@infradead.org \
--cc=will@kernel.org \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox