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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Dec 20, 2024 at 01:16:29AM +0800, Jinliang Zheng wrote:
> 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.

Does XFS pass buffers between tasks?  xfs_btree_split has that whole
blob of ugly code where it can pass a locked inode and transaction to a
workqueue function to avoid overrunning the kernel stack.

--D

> Signed-off-by: Jinliang Zheng <alexjlzheng@xxxxxxxxxxx>
> ---
>  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
> 
> 




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux