[PATCH] xfs: using mutex instead of semaphore for xfs_buf_lock()

Jinliang Zheng
2024-12-19T17:16:35Z

xfs_buf uses a semaphore for mutual exclusion, and its count value
is initialized to 1, which is equivalent to a mutex.

However, mutex->owner can provide more information when analyzing
vmcore, making it easier for us to identify which task currently
holds the lock.

Signed-off-by: Jinliang Zheng <alexjlzheng@tencent.com>
---
fs/xfs/xfs_buf.c | 9 +++++----
fs/xfs/xfs_buf.h | 4 ++--
fs/xfs/xfs_trace.h | 25 +++++--------------------
3 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index aa4dbda7b536..7c59d7905ea1 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -243,7 +243,8 @@ _xfs_buf_alloc(
INIT_LIST_HEAD(&bp->b_lru);
INIT_LIST_HEAD(&bp->b_list);
INIT_LIST_HEAD(&bp->b_li_list);
- sema_init(&bp->b_sema, 0); /* held, no waiters */
+ mutex_init(&bp->b_mutex);
+ mutex_lock(&bp->b_mutex); /* held, no waiters */
spin_lock_init(&bp->b_lock);
bp->b_target = target;
bp->b_mount = target->bt_mount;
@@ -1168,7 +1169,7 @@ xfs_buf_trylock(
{
int locked;

- locked = down_trylock(&bp->b_sema) == 0;
+ locked = mutex_trylock(&bp->b_mutex);
if (locked)
trace_xfs_buf_trylock(bp, _RET_IP_);
else
@@ -1193,7 +1194,7 @@ xfs_buf_lock(

if (atomic_read(&bp->b_pin_count) && (bp->b_flags & XBF_STALE))
xfs_log_force(bp->b_mount, 0);
- down(&bp->b_sema);
+ mutex_lock(&bp->b_mutex);

trace_xfs_buf_lock_done(bp, _RET_IP_);
}
@@ -1204,7 +1205,7 @@ xfs_buf_unlock(
{
ASSERT(xfs_buf_islocked(bp));

- up(&bp->b_sema);
+ mutex_unlock(&bp->b_mutex);
trace_xfs_buf_unlock(bp, _RET_IP_);
}

diff --git a/fs/xfs/xfs_buf.h b/fs/xfs/xfs_buf.h
index b1580644501f..2c48e388d451 100644
--- a/fs/xfs/xfs_buf.h
+++ b/fs/xfs/xfs_buf.h
@@ -171,7 +171,7 @@ struct xfs_buf {
atomic_t b_hold; /* reference count */
atomic_t b_lru_ref; /* lru reclaim ref count */
xfs_buf_flags_t b_flags; /* status flags */
- struct semaphore b_sema; /* semaphore for lockables */
+ struct mutex b_mutex; /* mutex for lockables */

/*
* concurrent access to b_lru and b_lru_flags are protected by
@@ -304,7 +304,7 @@ extern int xfs_buf_trylock(struct xfs_buf *);
extern void xfs_buf_lock(struct xfs_buf *);
extern void xfs_buf_unlock(struct xfs_buf *);
#define xfs_buf_islocked(bp) \
- ((bp)->b_sema.count <= 0)
+ mutex_is_locked(&(bp)->b_mutex)

static inline void xfs_buf_relse(struct xfs_buf *bp)
{
diff --git a/fs/xfs/xfs_trace.h b/fs/xfs/xfs_trace.h
index 180ce697305a..ba6c003b82af 100644
--- a/fs/xfs/xfs_trace.h
+++ b/fs/xfs/xfs_trace.h
@@ -443,7 +443,6 @@ DECLARE_EVENT_CLASS(xfs_buf_class,
__field(int, nblks)
__field(int, hold)
__field(int, pincount)
- __field(unsigned, lockval)
__field(unsigned, flags)
__field(unsigned long, caller_ip)
__field(const void *, buf_ops)
@@ -454,19 +453,17 @@ DECLARE_EVENT_CLASS(xfs_buf_class,
__entry->nblks = bp->b_length;
__entry->hold = atomic_read(&bp->b_hold);
__entry->pincount = atomic_read(&bp->b_pin_count);
- __entry->lockval = bp->b_sema.count;
__entry->flags = bp->b_flags;
__entry->caller_ip = caller_ip;
__entry->buf_ops = bp->b_ops;
),
TP_printk("dev %d:%d daddr 0x%llx bbcount 0x%x hold %d pincount %d "
- "lock %d flags %s bufops %pS caller %pS",
+ "flags %s bufops %pS caller %pS",
MAJOR(__entry->dev), MINOR(__entry->dev),
(unsigned long long)__entry->bno,
__entry->nblks,
__entry->hold,
__entry->pincount,
- __entry->lockval,
__print_flags(__entry->flags, "|", XFS_BUF_FLAGS),
__entry->buf_ops,
(void *)__entry->caller_ip)
@@ -514,7 +511,6 @@ DECLARE_EVENT_CLASS(xfs_buf_flags_class,
__field(unsigned int, length)
__field(int, hold)
__field(int, pincount)
- __field(unsigned, lockval)
__field(unsigned, flags)
__field(unsigned long, caller_ip)
),
@@ -525,17 +521,15 @@ DECLARE_EVENT_CLASS(xfs_buf_flags_class,
__entry->flags = flags;
__entry->hold = atomic_read(&bp->b_hold);
__entry->pincount = atomic_read(&bp->b_pin_count);
- __entry->lockval = bp->b_sema.count;
__entry->caller_ip = caller_ip;
),
TP_printk("dev %d:%d daddr 0x%llx bbcount 0x%x hold %d pincount %d "
- "lock %d flags %s caller %pS",
+ "flags %s caller %pS",
MAJOR(__entry->dev), MINOR(__entry->dev),
(unsigned long long)__entry->bno,
__entry->length,
__entry->hold,
__entry->pincount,
- __entry->lockval,
__print_flags(__entry->flags, "|", XFS_BUF_FLAGS),
(void *)__entry->caller_ip)
)
@@ -558,7 +552,6 @@ TRACE_EVENT(xfs_buf_ioerror,
__field(unsigned, flags)
__field(int, hold)
__field(int, pincount)
- __field(unsigned, lockval)
__field(int, error)
__field(xfs_failaddr_t, caller_ip)
),
@@ -568,19 +561,17 @@ TRACE_EVENT(xfs_buf_ioerror,
__entry->length = bp->b_length;
__entry->hold = atomic_read(&bp->b_hold);
__entry->pincount = atomic_read(&bp->b_pin_count);
- __entry->lockval = bp->b_sema.count;
__entry->error = error;
__entry->flags = bp->b_flags;
__entry->caller_ip = caller_ip;
),
TP_printk("dev %d:%d daddr 0x%llx bbcount 0x%x hold %d pincount %d "
- "lock %d error %d flags %s caller %pS",
+ "error %d flags %s caller %pS",
MAJOR(__entry->dev), MINOR(__entry->dev),
(unsigned long long)__entry->bno,
__entry->length,
__entry->hold,
__entry->pincount,
- __entry->lockval,
__entry->error,
__print_flags(__entry->flags, "|", XFS_BUF_FLAGS),
(void *)__entry->caller_ip)
@@ -595,7 +586,6 @@ DECLARE_EVENT_CLASS(xfs_buf_item_class,
__field(unsigned int, buf_len)
__field(int, buf_hold)
__field(int, buf_pincount)
- __field(int, buf_lockval)
__field(unsigned, buf_flags)
__field(unsigned, bli_recur)
__field(int, bli_refcount)
@@ -612,18 +602,16 @@ DECLARE_EVENT_CLASS(xfs_buf_item_class,
__entry->buf_flags = bip->bli_buf->b_flags;
__entry->buf_hold = atomic_read(&bip->bli_buf->b_hold);
__entry->buf_pincount = atomic_read(&bip->bli_buf->b_pin_count);
- __entry->buf_lockval = bip->bli_buf->b_sema.count;
__entry->li_flags = bip->bli_item.li_flags;
),
TP_printk("dev %d:%d daddr 0x%llx bbcount 0x%x hold %d pincount %d "
- "lock %d flags %s recur %d refcount %d bliflags %s "
+ "flags %s recur %d refcount %d bliflags %s "
"liflags %s",
MAJOR(__entry->dev), MINOR(__entry->dev),
(unsigned long long)__entry->buf_bno,
__entry->buf_len,
__entry->buf_hold,
__entry->buf_pincount,
- __entry->buf_lockval,
__print_flags(__entry->buf_flags, "|", XFS_BUF_FLAGS),
__entry->bli_recur,
__entry->bli_refcount,
@@ -4802,7 +4790,6 @@ DECLARE_EVENT_CLASS(xfbtree_buf_class,
__field(int, nblks)
__field(int, hold)
__field(int, pincount)
- __field(unsigned int, lockval)
__field(unsigned int, flags)
),
TP_fast_assign(
@@ -4811,16 +4798,14 @@ DECLARE_EVENT_CLASS(xfbtree_buf_class,
__entry->nblks = bp->b_length;
__entry->hold = atomic_read(&bp->b_hold);
__entry->pincount = atomic_read(&bp->b_pin_count);
- __entry->lockval = bp->b_sema.count;
__entry->flags = bp->b_flags;
),
- TP_printk("xfino 0x%lx daddr 0x%llx bbcount 0x%x hold %d pincount %d lock %d flags %s",
+ TP_printk("xfino 0x%lx daddr 0x%llx bbcount 0x%x hold %d pincount %d flags %s",
__entry->xfino,
(unsigned long long)__entry->bno,
__entry->nblks,
__entry->hold,
__entry->pincount,
- __entry->lockval,
__print_flags(__entry->flags, "|", XFS_BUF_FLAGS))
)

--
2.41.1