On 2/23/20 5:42 AM, Amir Goldstein wrote:
On Sun, Feb 23, 2020 at 4:07 AM Allison Collins
<allison.henderson@xxxxxxxxxx> wrote:
Factor out new helper function xfs_attr_leaf_try_add.
Right. This should be the subject.
Not factor out xfs_attr_leaf_addname helper.
Because new delayed attribute
routines cannot roll transactions, we carve off the parts of xfs_attr_leaf_addname
that we can use, and move the commit into the calling function.
And that is a different change.
It is hard enough to review a pure factor out of a helper.
Adding changed inside a pure re-factor is not good.
Belongs to another change - move transaction commit to caller.
Yes, this came up in the last review, but the reason I avoid it is
because I think the transaction looks weird in either helper. The
reason the roll is here is because there is no room to add the attr as a
leaf, and so we need to hand it off to the node subroutines. But that
seems like something that should be managed by the caller, not leaf
helpers. So I was concerned that separating the split and the hoist
would generate more weird looks from people trying to understand the
split until the see the hoist in the next patch. If people really think
it looks better that way, I can split them up. But I know folks have a
tough enough time trying to recall the discussion history, so I'm trying
to avoid confusion of another type :-)
Thoughts?
Allison
Thanks,
Amir.
