Re: [PATCH] xfs: don't hold buffer locks across sync transaction commit in xfs_sync_sb_buf

[Darrick J. Wong]

On Mon, Jul 13, 2026 at 10:50:02PM +0800, Yun Zhou wrote:
> xfs_sync_sb_buf() holds sb/rtsb buffer locks across a synchronous
> xfs_trans_commit(), which flushes the CIL push workqueue internally.
> If shutdown occurs during the CIL push, xfs_buf_item_unpin() needs to
> lock these buffers to fail them, causing a deadlock:
> 
>   setlabel:          holds buf lock -> flush_workqueue(xfs-cil)
>   CIL push worker:   xfs_buf_item_unpin -> xfs_buf_lock(same buf)
> 
> Remove the xfs_trans_bhold() calls so that commit releases the buffer
> locks normally.  After the sync commit, re-acquire the buffers via
> mp->m_sb_bp / mp->m_rtsb_bp for the on-disk writeback.

Is there a risk of writing out the wrong superblock contents if
something else grabs the buffer lock(s) between the xfs_trans_commit and
the xfs_buf_lock calls?  Can we walk off a garbage
xfs_mount::m_{rt,}sb_bp pointer if the filesystem is being torn down, or
does something prevent that?

--D

> Fixes: f7664b31975b ("xfs: implement online get/set fs label")
> Reported-by: syzbot+837bcd54843dd6262f2f@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=837bcd54843dd6262f2f
> Signed-off-by: Yun Zhou <yun.zhou@xxxxxxxxxxxxx>
> ---
>  fs/xfs/libxfs/xfs_sb.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
> index 47322adb7690..fbb4505b1e9e 100644
> --- a/fs/xfs/libxfs/xfs_sb.c
> +++ b/fs/xfs/libxfs/xfs_sb.c
> @@ -1471,7 +1471,6 @@ xfs_sync_sb_buf(
>  {
>  	struct xfs_trans	*tp;
>  	struct xfs_buf		*bp;
> -	struct xfs_buf		*rtsb_bp = NULL;
>  	int			error;
>  
>  	error = xfs_trans_alloc(mp, &M_RES(mp)->tr_sb, 0, 0, 0, &tp);
> @@ -1480,26 +1479,28 @@ xfs_sync_sb_buf(
>  
>  	bp = xfs_trans_getsb(tp);
>  	xfs_log_sb(tp);
> -	xfs_trans_bhold(tp, bp);
> -	if (update_rtsb) {
> -		rtsb_bp = xfs_log_rtsb(tp, bp);
> -		if (rtsb_bp)
> -			xfs_trans_bhold(tp, rtsb_bp);
> -	}
> +	if (update_rtsb)
> +		xfs_log_rtsb(tp, bp);
>  	xfs_trans_set_sync(tp);
>  	error = xfs_trans_commit(tp);
>  	if (error)
> -		goto out;
> -	/*
> -	 * write out the sb buffer to get the changes to disk
> -	 */
> -	error = xfs_bwrite(bp);
> -	if (!error && rtsb_bp)
> -		error = xfs_bwrite(rtsb_bp);
> -out:
> -	if (rtsb_bp)
> -		xfs_buf_relse(rtsb_bp);
> -	xfs_buf_relse(bp);
> +		return error;
> +
> +	/* Re-acquire and write the sb and rtsb to disk. */
> +	xfs_buf_lock(mp->m_sb_bp);
> +	xfs_buf_hold(mp->m_sb_bp);
> +	error = xfs_bwrite(mp->m_sb_bp);
> +	xfs_buf_relse(mp->m_sb_bp);
> +	if (error)
> +		return error;
> +
> +	if (update_rtsb && mp->m_rtsb_bp) {
> +		xfs_buf_lock(mp->m_rtsb_bp);
> +		xfs_buf_hold(mp->m_rtsb_bp);
> +		error = xfs_bwrite(mp->m_rtsb_bp);
> +		xfs_buf_relse(mp->m_rtsb_bp);
> +	}
> +
>  	return error;
>  }
>  
> -- 
> 2.43.0
> 
>