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

[Darrick J. Wong]

On Tue, Jul 14, 2026 at 09:41:11AM +0800, Zhou, Yun wrote:
> 
> 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().

<nod> Ok then.

Reviewed-by: "Darrick J. Wong" <djwong@xxxxxxxxxx>

--D


> BR,
> Yun
>