From: "Darrick J. Wong" <djwong@kernel.org>
To: Carlos Maiolino <cem@kernel.org>
Cc: Christoph Hellwig <hch@infradead.org>,
Aditya Prakash Srivastava <aditya.ansh182@gmail.com>,
linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v5 2/2] xfs: prevent close() from hanging on frozen filesystems
Date: Fri, 14 Aug 2026 11:24:49 -0700 [thread overview]
Message-ID: <20260814182449.GI3556460@frogsfrogsfrogs> (raw)
In-Reply-To: <an9BQdSqrmzYhB3l@andromeda.toxiclabs.cc>
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
prev parent reply other threads:[~2026-08-14 18:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-06-16 5:38 [PATCH v5 0/2] xfs: resolve close() deadlocks on frozen filesystems Aditya Srivastava
2026-06-16 5:38 ` [PATCH v5 1/2] xfs: add a XFS_TRANS_WRITECOUNT_TRYLOCK flag Aditya Srivastava
2026-06-16 5:38 ` [PATCH v5 2/2] xfs: prevent close() from hanging on frozen filesystems Aditya Srivastava
2026-06-16 13:04 ` Christoph Hellwig
2026-06-24 17:35 ` Darrick J. Wong
2026-06-24 18:28 ` Aditya Prakash Srivastava
2026-07-08 13:19 ` Aditya Prakash Srivastava
2026-08-13 16:13 ` Aditya Prakash Srivastava
2026-08-14 7:28 ` Christoph Hellwig
2026-08-14 16:25 ` Carlos Maiolino
2026-08-14 18:24 ` Darrick J. Wong [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260814182449.GI3556460@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=aditya.ansh182@gmail.com \
--cc=cem@kernel.org \
--cc=hch@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox