Re: [PATCH v5 2/2] xfs: prevent close() from hanging on frozen filesystems
"Darrick J. Wong" <[email protected]>
| Newsgroups | org.kernel.vger.linux-xfs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <20260814182449.GI3556460@frogsfrogsfrogs> |
On Fri, Aug 14, 2026 at 06:25:41PM +0200, Carlos Maiolino wrote:
> On Fri, Aug 14, 2026 at 12:28:19AM -0700, Christoph Hellwig wrote:
> > On Thu, Aug 13, 2026 at 09:43:44PM +0530, Aditya Prakash Srivastava wrote:
> > >
> > > Are there any remaining concerns or feedback that need
> > > to be addressed with this patch?
> >
> > Still looks good to me, but I think it just go lost. It's a bit close
> > for the 7.3 merge window now, but I'd suggest you resend it in a week
> > or two to get it onto the agenda for 7.4 early.
>
> Yup, I was waiting for Darrick to follow-up then it fell on limbo.
> Please follow Christoph's advice, please rebase and re-send on top of
> the tree after 7.3 merge window.
I ignored the response because it's clearly AI slop, and obviously
wrong.
"In XFS, the canonical locking hierarchy is:
"sb_start_write (freeze lock) -> IOLOCK -> ILOCK ->
xfs_trans_alloc"
Nope.
"xfs_file_release() already holds XFS_IOLOCK_EXCL (acquired via
xfs_ilock_nowait())..."
Nope. The snippet I suggested isn't fully formed, but this is obviously
wrong; go read xfs_file_release.
static inline void xfs_file_release_eofblocks(struct xfs_inode *ip)
{
if (!xfs_can_free_eofblocks(ip))
return;
if (xfs_iflags_test(ip, XFS_EOFBLOCKS_RELEASED))
return;
/* take freeze protection and i_rwsem */
if (!sb_start_write_trylock(...))
return;
if (!xfs_ilock_nowait(ip, XFS_IOLOCK_EXCL))
goto out_sb_write;
/*
* recheck if we want to trim eofblocks and someone else hasn't
* already done this
*/
if (!xfs_can_free_eofblocks(ip))
goto out_iolock;
if (xfs_iflags_test_and_set(ip, XFS_EOFBLOCKS_RELEASED))
goto out_iolock;
xfs_free_eofblocks(ip);
out_iolock:
xfs_iunlock(ip, XFS_IOLOCK_EXCL);
out_sb_write:
sb_end_write(...);
}
"xfs_trans_alloc() is the canonical, centralized place in XFS to
manage write-count blocking semantics."
Nope. This isn't even talking about the same vfs lock here.
--D