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

[Zhou, Yun]

Thanks a lot for your review.

On 7/14/26 06:04, Darrick J. Wong wrote:
> 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?

There shouldn't be a risk of inconsistent contents. Each update to the 
sb buffer is a full overwrite of mp->m_sb via xfs_sb_to_disk(), 
performed under the buffer lock. So when we re-lock after commit, the 
buffer  always contains a complete, consistent snapshot — we may see a 
newer version if someone else updated it in between, but never a 
partially written one.

For the pointer lifetime: mp->m_sb_bp is allocated at mount and freed in 
xfs_unmountfs(). The caller holds mnt_want_write_file(), which prevents 
unmount from making progress, so the pointer should remain valid 
throughout xfs_sync_sb_buf().

BR,
Yun