[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aYTQyseO0dEjfPIp@nidhogg.toxiclabs.cc>
Date: Thu, 5 Feb 2026 18:19:32 +0100
From: Carlos Maiolino <cem@...nel.org>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Christoph Hellwig <hch@...radead.org>, zlang@...hat.com,
linux-ext4@...r.kernel.org, fstests@...r.kernel.org
Subject: Re: [PATCH 1/3] xfs/018: remove inline xattr recovery tests
On Thu, Feb 05, 2026 at 08:56:24AM -0800, Darrick J. Wong wrote:
> On Mon, Feb 02, 2026 at 09:22:36PM -0800, Christoph Hellwig wrote:
> > On Mon, Feb 02, 2026 at 11:11:12AM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@...nel.org>
> > >
FWIW, this fix the regressions I'm hitting on 7.0 merge window queue.
> > > Now that we can do xattr updates in a single transaction (as opposed to
> > > using the attr intent machinery) if we keep the attr structure in short
> > > format, remove the attr intent item log recovery tests.
> >
> > I have a bit of a hard time parsing this. Currently with xfs/for-next
> > these fail, so removing them fixes it, which is probably what drove
> > this.
>
> Yep.
>
> > But looking through the patches I'm not sure why they actually are
> > failing - the updates are logged as part of the inode item, and
> > nothing in test_attr_replay seems to actually look at log specific
> > bits?
>
> I've rewritten the commit message; does this help?
>
> "In Linux 7.0 we've changed the extended attribute update code to try to
> take a shortcut for performance reasons. Before walking through the
> attr intent state machine (slow), the update will check to see if the
> attr structure is in short format and will stay in that format after the
> change. If so, then the incore inode can be updated and logged, and the
> update is complete (fast) in a single transaction.
>
> "(Obviously, for complex attr structures or large changes we still walk
> through the intent machinery.)
>
> "However, xfs/018 tests the behavior of the "larp" error injector, which
> only triggers from inside the attr intent state machine. Therefore, the
> short format tests don't actually trip the injector. It makes no sense
> to add a new larp injection callsite for the shortcut because either the
> single transaction gets written to disk or it doesn't."
>
> > Only vaguely related, but should we ensure to always clear error
> > tags after the test runs to ensure they don't leak into other tests?
>
> They go away with _scratch_remount because error tags only live as long
> as the mount.
The changes looks good to me, if you re-send this, feel free to add:
Reviewed-by: Carlos Maiolino <cmaiolino@...hat.com>
>
> --D
>
Powered by blists - more mailing lists