On 11/14/23 16:26, Matthew Wilcox wrote:
On Fri, Nov 10, 2023 at 05:21:22PM -0500, Waiman Long wrote:
On 11/10/23 15:41, Matthew Wilcox (Oracle) wrote:
static inline int rwsem_is_locked(struct rw_semaphore *sem)
{
- return atomic_long_read(&sem->count) != 0;
+ return atomic_long_read(&sem->count) != RWSEM_UNLOCKED_VALUE;
}
-#define RWSEM_UNLOCKED_VALUE 0L
-#define __RWSEM_COUNT_INIT(name) .count = ATOMIC_LONG_INIT(RWSEM_UNLOCKED_VALUE)
+static inline void rwsem_assert_held_nolockdep(const struct rw_semaphore *sem)
+{
+ WARN_ON(atomic_long_read(&sem->count) == RWSEM_UNLOCKED_VALUE);
+}
That is not correct. You mean "!= RWSEM_UNLOCKED_VALUE". Right?
Uhhh ... I always get confused between assert and BUG_ON being opposite
polarity, but I think it's correct.
We are asserting that the rwsem is locked (either for read or write).
That is, it is a bug if the rwsem is unlocked.
So WARN_ON(sem->count == UNLOCKED_VALUE) is correct. No?
You are right. I got confused too.
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.
BTW, we can actually check if the current process is the write-lock owner of
a rwsem, but not for a reader-owned rwsem.
We actually don't want to do that. See patches 3/4 where I explain how
XFS takes the XFS_ILOCK for write, then passes control to a workqueue
which asserts that the XFS_ILOCK is held for write. The thread which
took the rwsem for write waits for the workqueue and unlocks the rwsem.
I see. Thanks for the explanation.
Cheers,
Longman
