All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10 v2] xfs: LARP - clean up xfs_attr_set_iter state machine
@ 2022-04-14  9:44 Dave Chinner
  2022-04-14  9:44 ` [PATCH 01/16] xfs: avoid empty xattr transaction when attrs are inline Dave Chinner
                   ` (15 more replies)
  0 siblings, 16 replies; 40+ messages in thread
From: Dave Chinner @ 2022-04-14  9:44 UTC (permalink / raw)
  To: linux-xfs

HI folks,

Version 2 is here and working better. The first 11 patches are
largely the same as the first version, just we a couple of bugs
fixes which caused a couple patches to be reworked to fix conflicts.
These patches set up the xfs_attr_set_iter() state machine to be
more readable and maintainable. It largely jsut worked with LARP=0,
but failed on the first recovery when LARP was enabled.

I realised that recovery wasn't setting the initial state properly,
and then started digging into the recovery code. At this point, I
realised the recovery code didn't work correctly in all cases and
could often leave unremovable incomplete attrs sitting around. The
issues are larger documented ini patch 16 so I won't go over them
here, just read that patch.

However, to get, the whole replace operation for LARP=1 needed to
change. Luckily, that turned out to be pretty simple because it was
largely already broken down into component operations in the state
machine. hence I just needed to add new "remove" initial states to
the set_iter state machine, and that allowed the new algorithm to
function.

Then I realised that I'd just implemented the remove_iter algorithm
in the set_iter state machine, so I removed the remove_iter code and
it's states altogether and just pointed remove ops at the set-iter
remove initial states. The code now uses the XFS_DA_OP_RENAME flag
to determine if it should follow up an add or remove with a remove
or add, and it all largely jsut works. All runtime algorithms run
throught he same state machine just with different initial states
and state progressions.

And with patch 16, attr item log recovery uses that same state
machine, too. It has a few quirks that need to be handled, so I
added the XFS_DA_OP_RECOVERY flag to allow the right thing to be
done with the INCOMPLETE flag deep in the guts of the attr lookup
code. And so recovery with LARP=1 now seems to mostly work.

The current state is that larp=0 seems to pass attr group and
recoverloop group testing just fine. My current build is about 10
loops in without a failure. With larp=1, attr group passes jsut
fine, but I'm getting random recovery failures with recoveryloop
testing that aren't immediately obvious as being attribute intent
recvoery failures.  (e.g. things like unlinked inode recovery
finding inodes on the unlinked list that have nlink != 0). Hence
while it mostly works, I'm sure there are still some bugs lurking
there.

There's also a fair bit of cleanup needed of the last couple of
patches. There's some quirks in the state change code when
completing an op and determining whether we need to run the second
half of a replace op that need cleaning up, and the last patch needs
further splitting up and it changes stuff all over the place.

However, if you want to get an idea of what the code looks like,
see if it runs and breaks on your machines and maybe find some bugs
in it, help is welcome!

Cheers,

Dave.



^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2022-05-04  5:49 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-14  9:44 [PATCH 00/10 v2] xfs: LARP - clean up xfs_attr_set_iter state machine Dave Chinner
2022-04-14  9:44 ` [PATCH 01/16] xfs: avoid empty xattr transaction when attrs are inline Dave Chinner
2022-04-22  0:37   ` Alli
2022-04-14  9:44 ` [PATCH 02/16] xfs: initialise attrd item to zero Dave Chinner
2022-04-22  0:37   ` Alli
2022-04-14  9:44 ` [PATCH 03/16] xfs: make xattri_leaf_bp more useful Dave Chinner
2022-04-22  0:37   ` Alli
2022-04-14  9:44 ` [PATCH 04/16] xfs: separate out initial attr_set states Dave Chinner
2022-04-22  0:38   ` Alli
2022-04-26 23:50     ` Dave Chinner
2022-04-14  9:44 ` [PATCH 05/16] xfs: kill XFS_DAC_LEAF_ADDNAME_INIT Dave Chinner
2022-04-22  0:38   ` Alli
2022-04-14  9:44 ` [PATCH 06/16] xfs: consolidate leaf/node states in xfs_attr_set_iter Dave Chinner
2022-04-23  1:05   ` Alli
2022-04-14  9:44 ` [PATCH 07/16] xfs: split remote attr setting out from replace path Dave Chinner
2022-04-23  1:06   ` Alli
2022-04-14  9:44 ` [PATCH 08/16] xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP Dave Chinner
2022-04-23  1:06   ` Alli
2022-04-14  9:44 ` [PATCH 09/16] xfs: remote xattr removal in xfs_attr_set_iter() is conditional Dave Chinner
2022-04-23  1:07   ` Alli
2022-04-14  9:44 ` [PATCH 10/16] xfs: clean up final attr removal in xfs_attr_set_iter Dave Chinner
2022-04-23  1:06   ` Alli
2022-04-27  0:32   ` Alli
2022-04-14  9:44 ` [PATCH 11/16] xfs: xfs_attr_set_iter() does not need to return EAGAIN Dave Chinner
2022-04-27  0:32   ` Alli
2022-04-14  9:44 ` [PATCH 12/16] xfs: introduce attr remove initial states into xfs_attr_set_iter Dave Chinner
2022-04-27  0:33   ` Alli
2022-04-14  9:44 ` [PATCH 13/16] xfs: switch attr remove to xfs_attri_set_iter Dave Chinner
2022-04-27  0:34   ` Alli
2022-05-03  6:43     ` Dave Chinner
2022-04-14  9:44 ` [PATCH 14/16] xfs: remove xfs_attri_remove_iter Dave Chinner
2022-04-27  0:34   ` Alli
2022-05-03  6:53     ` Dave Chinner
2022-04-14  9:44 ` [PATCH 15/16] xfs: split attr replace op setup from create op setup Dave Chinner
2022-04-27  1:10   ` Alli
2022-04-14  9:44 ` [PATCH 16/16] xfs: ATTR_REPLACE algorithm with LARP enabled needs rework Dave Chinner
2022-04-28  7:02   ` Alli
2022-05-03  7:40     ` Dave Chinner
2022-05-04  1:30       ` Alli
2022-05-04  5:49         ` Dave Chinner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.