Re: [PATCH v7 13/19] xfs: Add delay ready attr remove routines

Allison Collins
2020-02-26T19:25:51Z

On 2/26/20 6:48 AM, Brian Foster wrote:
> On Tue, Feb 25, 2020 at 10:36:18PM -0700, Allison Collins wrote:
>>
>>
>> On 2/25/20 6:34 AM, Brian Foster wrote:
>>> On Mon, Feb 24, 2020 at 04:14:48PM -0700, Allison Collins wrote:
>>>> On 2/24/20 8:25 AM, Brian Foster wrote:
>>>>> On Sat, Feb 22, 2020 at 07:06:05PM -0700, Allison Collins wrote:
>>>>>> This patch modifies the attr remove routines to be delay ready. This means they no
>>>>>> longer roll or commit transactions, but instead return -EAGAIN to have the calling
>>>>>> routine roll and refresh the transaction. In this series, xfs_attr_remove_args has
>>>>>> become xfs_attr_remove_iter, which uses a sort of state machine like switch to keep
>>>>>> track of where it was when EAGAIN was returned. xfs_attr_node_removename has also
>>>>>> been modified to use the switch, and a new version of xfs_attr_remove_args
>>>>>> consists of a simple loop to refresh the transaction until the operation is
>>>>>> completed.
>>>>>>
>>>>>> This patch also adds a new struct xfs_delattr_context, which we will use to keep
>>>>>> track of the current state of an attribute operation. The new xfs_delattr_state
>>>>>> enum is used to track various operations that are in progress so that we know not
>>>>>> to repeat them, and resume where we left off before EAGAIN was returned to cycle
>>>>>> out the transaction. Other members take the place of local variables that need
>>>>>> to retain their values across multiple function recalls.
>>>>>>
>>>>>> Below is a state machine diagram for attr remove operations. The XFS_DAS_* states
>>>>>> indicate places where the function would return -EAGAIN, and then immediately
>>>>>> resume from after being recalled by the calling function. States marked as a
>>>>>> "subroutine state" indicate that they belong to a subroutine, and so the calling
>>>>>> function needs to pass them back to that subroutine to allow it to finish where
>>>>>> it left off. But they otherwise do not have a role in the calling function other
>>>>>> than just passing through.
>>>>>>
>>>>>> xfs_attr_remove_iter()
>>>>>> XFS_DAS_RM_SHRINK ─┐
>>>>>> (subroutine state) │
>>>>>> │
>>>>>> XFS_DAS_RMTVAL_REMOVE ─┤
>>>>>> (subroutine state) │
>>>>>> └─>xfs_attr_node_removename()
>>>>>> │
>>>>>> v
>>>>>> need to remove
>>>>>> ┌─n── rmt blocks?
>>>>>> │ │
>>>>>> │ y
>>>>>> │ │
>>>>>> │ v
>>>>>> │ ┌─>XFS_DAS_RMTVAL_REMOVE
>>>>>> │ │ │
>>>>>> │ │ v
>>>>>> │ └──y── more blks
>>>>>> │ to remove?
>>>>>> │ │
>>>>>> │ n
>>>>>> │ │
>>>>>> │ v
>>>>>> │ need to
>>>>>> └─────> shrink tree? ─n─┐
>>>>>> │ │
>>>>>> y │
>>>>>> │ │
>>>>>> v │
>>>>>> XFS_DAS_RM_SHRINK │
>>>>>> │ │
>>>>>> v │
>>>>>> done <─────┘
>>>>>>
>>>>>
>>>>> Wow. :P I guess I have nothing against verbose commit logs, but I wonder
>>>>> how useful this level of documentation is for a patch that shouldn't
>>>>> really change the existing flow of the operation.
>>>>
>>>> Yes Darrick had requested a diagram in the last review, so I had put this
>>>> together. I wasnt sure where the best place to put it even was, so I put it
>>>> here at least for now. I have no idea if there is a limit on commit message
>>>> length, but if there is, I'm pretty sure I blew right past it in this patch
>>>> and the next. Maybe if anything it can just be here for now while we work
>>>> through things?
>>>>
>>>
>>> No problem.. if it's useful it's good to have a record of out around
>>> somewhere until the end result is more stabilized and we can determine
>>> whether this warrants a permanent home somewhere in the code.
>>>
>>>>>
>>>>>> Signed-off-by: Allison Collins <allison.henderson@oracle.com>
>>>>>> ---
>>>>>> fs/xfs/libxfs/xfs_attr.c | 114 +++++++++++++++++++++++++++++++++++++------
>>>>>> fs/xfs/libxfs/xfs_attr.h | 1 +
>>>>>> fs/xfs/libxfs/xfs_da_btree.h | 30 ++++++++++++
>>>>>> fs/xfs/scrub/common.c | 2 +
>>>>>> fs/xfs/xfs_acl.c | 2 +
>>>>>> fs/xfs/xfs_attr_list.c | 1 +
>>>>>> fs/xfs/xfs_ioctl.c | 2 +
>>>>>> fs/xfs/xfs_ioctl32.c | 2 +
>>>>>> fs/xfs/xfs_iops.c | 2 +
>>>>>> fs/xfs/xfs_xattr.c | 1 +
>>>>>> 10 files changed, 141 insertions(+), 16 deletions(-)
>>>>>>
>>>>>> diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
>>>>>> index 5d73bdf..cd3a3f7 100644
>>>>>> --- a/fs/xfs/libxfs/xfs_attr.c
>>>>>> +++ b/fs/xfs/libxfs/xfs_attr.c
>>>>>> @@ -368,11 +368,60 @@ xfs_has_attr(
>>>>>> */
>>>>>> int
>>>>>> xfs_attr_remove_args(
>>>>>> + struct xfs_da_args *args)
>>>>>> +{
>>>>>> + int error = 0;
>>>>>> + int err2 = 0;
>>>>>> +
>>>>>> + do {
>>>>>> + error = xfs_attr_remove_iter(args);
>>>>>> + if (error && error != -EAGAIN)
>>>>>> + goto out;
>>>>>> +
>>>>>
>>>>> I'm a little confused on the logic of this loop given that the only
>>>>> caller commits the transaction (which also finishes dfops). IOW, it
>>>>> seems we shouldn't ever need to finish/roll when error != -EAGAIN. If
>>>>> that is the case, this can be simplified to something like:
>>>> Well, we need to do it when error == -EAGAIN or 0, right? Which I think
>>>> better imitates the defer_finish routines. That's why a lot of the existing
>>>> code that just finishes off with a transaction just sort of gets sawed off
>>>> at the end. Otherwise they would need one more state just to return -EAGAIN
>>>> as the last thing they have to do. Did that make sense?
>>>>
>>>
>>> Hmm.. I could just be missing something or not far along enough in the
>>> series. Can you point me at an example of where we need to finish/roll
>>> before the caller of xfs_attr_remove_args() commits the transaction?
>>>
>> Ok, in looking for an example, realized all such examples appear in the next
>> patch ;-) So maybe we can get away with simplifying it in this patch.
>>
>
> Ah, Ok. Yeah, I think that would be best so long as it is correct, since
> right now at least we have separate xfs_attr_[set|remove]_args() loop
> functions and I didn't see any code that warranted the extra roll in the
> remove path.
>
>> For the next patch though, it's any place the roll/finish disappears, and an
>> "return -EAGAIN" does not. For example, at the end of
>> xfs_attr_leaf_addname.
>>
>
> I see, thanks. Hmmm... so I think that particular example is basically a
> programming pattern thing moreso than a functional requirement. I.e.,
> the current _clearflag() function clears the flag and rolls the
> transaction perhaps simply so it can be reliably used in different
> contexts. The use in the _addname() case is functionally spurious afaict
> because we roll the transaction only to make no further changes and then
> commit the final transaction in the higher level code.
>
> I could see leaving the loop as is if this were the case for every exit
> path back to xfs_attr_set_args(), but is that really the case? If not,
> haven't we introduced a spurious roll for any zero return back to the
> _args() function? I think it might be best to fix up the loop to not
> roll on error == 0, explicitly plumb in the -EAGAIN in those spurious
> cases like _addname() where we currently roll, and then come up with a
> follow up patch to remove the ones that end up as spurious. That way
> we're not conflating too much refactoring with functional change and can
> review/document the functional change independently (i.e., if removing
> one of those rolls ends up introducing a bug, we don't have to revert an
> entire refactoring patch to restore original behavior).
>
> Now that I think of it, the better option is probably to remove the
> xfs_trans_roll_inode() call from _addname() first, before these patches
> introduce the delay ready infrastructure, since it's already isolated as
> spurious at that point. That should be a simple patch with a
> clear/obvious explanation.
Alrighty then, got it. I will add these suggestions into the next
version. Thanks again for all the reviews!

Allison

>
> Brian
>
>>>>>
>>>>> int
>>>>> xfs_attr_remove_args(
>>>>> struct xfs_da_args *args)
>>>>> {
>>>>> int error;
>>>>>
>>>>> do {
>>>>> error = xfs_attr_remove_iter(args);
>>>>> if (error != -EAGAIN)
>>>>> break;
>>>>>
>>>>> if (args->dac.flags & XFS_DAC_FINISH_TRANS) {
>>>>> args->dac.flags &= ~XFS_DAC_FINISH_TRANS;
>>>>> error = xfs_defer_finish(&args->trans);
>>>>> if (error)
>>>>> break;
>>>>> }
>>>>>
>>>>> error = xfs_trans_roll_inode(&args->trans, args->dp);
>>>>> if (error)
>>>>> break;
>>>>> } while (true);
>>>>>
>>>>> return error;
>>>>> }
>>>>>
>>>>> That has the added benefit of eliminating the whole err2 pattern, which
>>>>> always strikes me as a landmine.
>>>>>
>>>>>> + if (args->dac.flags & XFS_DAC_FINISH_TRANS) {
>>>>>
>>>>> BTW, _FINISH_TRANS also seems misnamed given that we finish deferred
>>>>> operations, not necessarily the transaction. XFS_DAC_DEFER_FINISH?
>>>> Sure, will update
>>>>
>>>>>
>>>>>> + args->dac.flags &= ~XFS_DAC_FINISH_TRANS;
>>>>>> +
>>>>>> + err2 = xfs_defer_finish(&args->trans);
>>>>>> + if (err2) {
>>>>>> + error = err2;
>>>>>> + goto out;
>>>>>> + }
>>>>>> + }
>>>>>> +
>>>>>> + err2 = xfs_trans_roll_inode(&args->trans, args->dp);
>>>>>> + if (err2) {
>>>>>> + error = err2;
>>>>>> + goto out;
>>>>>> + }
>>>>>> +
>>>>>> + } while (error == -EAGAIN);
>>>>>> +out:
>>>>>> + return error;
>>>>>> +}
>>>>>> +
>>>>>> +/*
>>>>>> + * Remove the attribute specified in @args.
>>>>>> + *
>>>>>> + * This function may return -EAGAIN to signal that the transaction needs to be
>>>>>> + * rolled. Callers should continue calling this function until they receive a
>>>>>> + * return value other than -EAGAIN.
>>>>>> + */
>>>>>> +int
>>>>>> +xfs_attr_remove_iter(
>>>>>> struct xfs_da_args *args)
>>>>>> {
>>>>>> struct xfs_inode *dp = args->dp;
>>>>>> int error;
>>>>>> + /* State machine switch */
>>>>>> + switch (args->dac.dela_state) {
>>>>>> + case XFS_DAS_RM_SHRINK:
>>>>>> + case XFS_DAS_RMTVAL_REMOVE:
>>>>>> + goto node;
>>>>>> + default:
>>>>>> + break;
>>>>>> + }
>>>>>> +
>>>>>> if (!xfs_inode_hasattr(dp)) {
>>>>>> error = -ENOATTR;
>>>>>> } else if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL) {
>>>>>> @@ -381,6 +430,7 @@ xfs_attr_remove_args(
>>>>>> } else if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
>>>>>> error = xfs_attr_leaf_removename(args);
>>>>>> } else {
>>>>>> +node:
>>>>>> error = xfs_attr_node_removename(args);
>>>>>> }
>>>>>> @@ -895,9 +945,8 @@ xfs_attr_leaf_removename(
>>>>>> /* bp is gone due to xfs_da_shrink_inode */
>>>>>> if (error)
>>>>>> return error;
>>>>>> - error = xfs_defer_finish(&args->trans);
>>>>>> - if (error)
>>>>>> - return error;
>>>>>> +
>>>>>> + args->dac.flags |= XFS_DAC_FINISH_TRANS;
>>>>>> }
>>>>>> return 0;
>>>>>> }
>>>>>> @@ -1218,6 +1267,11 @@ xfs_attr_node_addname(
>>>>>> * This will involve walking down the Btree, and may involve joining
>>>>>> * leaf nodes and even joining intermediate nodes up to and including
>>>>>> * the root node (a special case of an intermediate node).
>>>>>> + *
>>>>>> + * This routine is meant to function as either an inline or delayed operation,
>>>>>> + * and may return -EAGAIN when the transaction needs to be rolled. Calling
>>>>>> + * functions will need to handle this, and recall the function until a
>>>>>> + * successful error code is returned.
>>>>>> */
>>>>>> STATIC int
>>>>>> xfs_attr_node_removename(
>>>>>> @@ -1230,10 +1284,24 @@ xfs_attr_node_removename(
>>>>>> struct xfs_inode *dp = args->dp;
>>>>>> trace_xfs_attr_node_removename(args);
>>>>>> + state = args->dac.da_state;
>>>>>> + blk = args->dac.blk;
>>>>>> +
>>>>>> + /* State machine switch */
>>>>>> + switch (args->dac.dela_state) {
>>>>>> + case XFS_DAS_RMTVAL_REMOVE:
>>>>>> + goto rm_node_blks;
>>>>>> + case XFS_DAS_RM_SHRINK:
>>>>>> + goto rm_shrink;
>>>>>> + default:
>>>>>> + break;
>>>>>> + }
>>>>>> error = xfs_attr_node_hasname(args, &state);
>>>>>> if (error != -EEXIST)
>>>>>> goto out;
>>>>>> + else
>>>>>> + error = 0;
>>>>>
>>>>> This doesn't look necessary.
>>>> Well, at this point error has to be -EEXIST. Which is great because we need
>>>> the attr to exist, but we dont want to return that as error for this
>>>> function. Which can happen if error is not otherwise set.
>>>>
>>>
>>> AFAICT every codepath after this assigns error one way or another before
>>> it's returned. There's another error = 0 assignment just before the out:
>>> label.
>> Ok, I see it. Will remove.
>>
>>>
>>>>>
>>>>>> /*
>>>>>> * If there is an out-of-line value, de-allocate the blocks.
>>>>>> @@ -1243,6 +1311,14 @@ xfs_attr_node_removename(
>>>>>> blk = &state->path.blk[ state->path.active-1 ];
>>>>>> ASSERT(blk->bp != NULL);
>>>>>> ASSERT(blk->magic == XFS_ATTR_LEAF_MAGIC);
>>>>>> +
>>>>>> + /*
>>>>>> + * Store blk and state in the context incase we need to cycle out the
>>>>>> + * transaction
>>>>>> + */
>>>>>> + args->dac.blk = blk;
>>>>>> + args->dac.da_state = state;
>>>>>> +
>>>>>> if (args->rmtblkno > 0) {
>>>>>> /*
>>>>>> * Fill in disk block numbers in the state structure
>>>>>> @@ -1261,13 +1337,21 @@ xfs_attr_node_removename(
>>>>>> if (error)
>>>>>> goto out;
>>>>>> - error = xfs_trans_roll_inode(&args->trans, args->dp);
>>>>>> + error = xfs_attr_rmtval_invalidate(args);
>>>>>
>>>>> Remind me why we lose the above trans roll? I vaguely recall that this
>>>>> was intentional, but I could be mistaken...
>>>> I think we removed it in v5. We used to have a XFS_DAS_RM_INVALIDATE
>>>> state, but then we reasoned that because these are just in-core changes, we
>>>> didnt need it, so we eliminated this state entirely.
>>>>
>>>> Maybe i just add a comment here? Just as a reminder
>>>>
>>>
>>> Ah, Ok. Normally I'd say document things like this in the commit log so
>>> we don't lose track, though I don't know how much space we have there.
>>> ;)
>> Ok, I'll see if I can squeeze in a few more lines :-)
>>
>>>
>>>>>
>>>>>> if (error)
>>>>>> goto out;
>>>>>> + }
>>>>>> - error = xfs_attr_rmtval_remove(args);
>>>>>> - if (error)
>>>>>> - goto out;
>>>>>> +rm_node_blks:
>>>>>> +
>>>>>> + if (args->rmtblkno > 0) {
>>>>>> + error = xfs_attr_rmtval_unmap(args);
>>>>>> +
>>>>>> + if (error) {
>>>>>> + if (error == -EAGAIN)
>>>>>> + args->dac.dela_state = XFS_DAS_RMTVAL_REMOVE;
>>>>>
>>>>> Might be helpful for the code labels to match the state names. I.e., use
>>>>> das_rmtval_remove: for the label above.
>>>> Sure, I can update add the das prefix.
>>>>
>>>>>
>>>>>> + return error;
>>>>>> + }
>>>>>> /*
>>>>>> * Refill the state structure with buffers, the prior calls
>>>>>> @@ -1293,17 +1377,15 @@ xfs_attr_node_removename(
>>>>>> error = xfs_da3_join(state);
>>>>>> if (error)
>>>>>> goto out;
>>>>>> - error = xfs_defer_finish(&args->trans);
>>>>>> - if (error)
>>>>>> - goto out;
>>>>>> - /*
>>>>>> - * Commit the Btree join operation and start a new trans.
>>>>>> - */
>>>>>> - error = xfs_trans_roll_inode(&args->trans, dp);
>>>>>> - if (error)
>>>>>> - goto out;
>>>>>> +
>>>>>> + args->dac.flags |= XFS_DAC_FINISH_TRANS;
>>>>>> + args->dac.dela_state = XFS_DAS_RM_SHRINK;
>>>>>> + return -EAGAIN;
>>>>>> }
>>>>>> +rm_shrink:
>>>>>> + args->dac.dela_state = XFS_DAS_RM_SHRINK;
>>>>>> +
>>>>>
>>>>> There's an xfs_defer_finish() call further down this function. Should
>>>>> that be replaced with the flag?
>>>>>
>>>>> Finally, I mentioned in a previous review that this function should
>>>>> probably be further broken down before fitting in the state management
>>>>> stuff. It doesn't look like that happened so I've attached a diff that
>>>>> is just intended to give an idea of what I mean by sectioning off the
>>>>> hunks that might be able to break down into helpers. The helpers
>>>>> wouldn't contain any state management, so we create a clear separation
>>>>> between the state code and functional components.
>>>> Yes, it's xfs_attr_node_shrink in patch 15. I moved it to another patch to
>>>> try and keep the activity in this one to a minimum. Apologies if it
>>>> surprised you! And then i mistakenly had taken the XFS_DAC_FINISH_TRANS
>>>> flag with it. I meant to keep all the state machine stuff here. Will fix!
>>>>
>>>
>>> Ok, I might have just not got there yet.
>>>
>>>> I think this initial
>>>>> refactoring would make the introduction of state much more simple
>>>>
>>>> I guess I didn't think people would be partial to introducing helpers before
>>>> or after the state logic. I put them after in this set because the states
>>>> are visible now, so I though it would make the goal of modularizing code
>>>> between the states more clear to folks. Do you think I should move it back
>>>> behind the state machine patches?
>>>>
>>>
>>> I do think the refactoring should be done first. This does make it more
>>> challenging for the developer (IMO) because I know I'd probably have to
>>> hack around with the state bits to have a better idea of how to refactor
>>> things in some cases, and then go back and retrofit the refactoring.
>>>
>>> The advantage is that the heavy lifting in this series becomes agnostic
>>> to the state bits. Refactoring patches are easier to review and we can
>>> make progress because there's less of a need to carry those out of tree
>>> through however many versions of the state code we'll need before
>>> getting it merged. Once the code is sufficiently factored, the state
>>> code should be much simpler to introduce and review since we hopefully
>>> won't be jumping around into the middle of functions, multiple branches
>>> of logic deep, etc.
>>>
>>> (I see Dave commented similarly on a couple of the subsequent patches. I
>>> 100% agree with the approach he describes there and that is similar to
>>> what I was trying to describe with the diff I attached in my earlier
>>> mail...)
>>>
>>> Brian
>>
>> Alrighty then, will move back. Thanks, and thanks again for the reviews!!
>>
>> Allison
>>
>>>
>>>> (and
>>>>> perhaps alleviate the need for the huge diagram).
>>>> Well, I get the impression that people find the series sort of scary and
>>>> maybe the diagrams help them a bit. Maybe we can take them out later after
>>>> people feel like they are comfortable with things?
>>>>
>>>> It might also be
>>>>> interesting to see how much of the result could be folded up further
>>>>> into _removename_iter()...
>>>>
>>>> Yes, I think that is the goal we're reaching for. I will add the other
>>>> helpers I see in your diff too.
>>>>
>>>> Thanks for the reviews!
>>>> Allison
>>>>
>>>>>
>>>>> Brian
>>>>>
>>>>>> /*
>>>>>> * If the result is small enough, push it all into the inode.
>>>>>> */
>>>>>> diff --git a/fs/xfs/libxfs/xfs_attr.h b/fs/xfs/libxfs/xfs_attr.h
>>>>>> index ce7b039..ea873a5 100644
>>>>>> --- a/fs/xfs/libxfs/xfs_attr.h
>>>>>> +++ b/fs/xfs/libxfs/xfs_attr.h
>>>>>> @@ -155,6 +155,7 @@ int xfs_attr_set_args(struct xfs_da_args *args);
>>>>>> int xfs_attr_remove(struct xfs_inode *dp, struct xfs_name *name, int flags);
>>>>>> int xfs_has_attr(struct xfs_da_args *args);
>>>>>> int xfs_attr_remove_args(struct xfs_da_args *args);
>>>>>> +int xfs_attr_remove_iter(struct xfs_da_args *args);
>>>>>> int xfs_attr_list(struct xfs_inode *dp, char *buffer, int bufsize,
>>>>>> int flags, struct attrlist_cursor_kern *cursor);
>>>>>> bool xfs_attr_namecheck(const void *name, size_t length);
>>>>>> diff --git a/fs/xfs/libxfs/xfs_da_btree.h b/fs/xfs/libxfs/xfs_da_btree.h
>>>>>> index 14f1be3..3c78498 100644
>>>>>> --- a/fs/xfs/libxfs/xfs_da_btree.h
>>>>>> +++ b/fs/xfs/libxfs/xfs_da_btree.h
>>>>>> @@ -50,9 +50,39 @@ enum xfs_dacmp {
>>>>>> };
>>>>>> /*
>>>>>> + * Enum values for xfs_delattr_context.da_state
>>>>>> + *
>>>>>> + * These values are used by delayed attribute operations to keep track of where
>>>>>> + * they were before they returned -EAGAIN. A return code of -EAGAIN signals the
>>>>>> + * calling function to roll the transaction, and then recall the subroutine to
>>>>>> + * finish the operation. The enum is then used by the subroutine to jump back
>>>>>> + * to where it was and resume executing where it left off.
>>>>>> + */
>>>>>> +enum xfs_delattr_state {
>>>>>> + XFS_DAS_RM_SHRINK, /* We are shrinking the tree */
>>>>>> + XFS_DAS_RMTVAL_REMOVE, /* We are removing remote value blocks */
>>>>>> +};
>>>>>> +
>>>>>> +/*
>>>>>> + * Defines for xfs_delattr_context.flags
>>>>>> + */
>>>>>> +#define XFS_DAC_FINISH_TRANS 0x1 /* indicates to finish the transaction */
>>>>>> +
>>>>>> +/*
>>>>>> + * Context used for keeping track of delayed attribute operations
>>>>>> + */
>>>>>> +struct xfs_delattr_context {
>>>>>> + struct xfs_da_state *da_state;
>>>>>> + struct xfs_da_state_blk *blk;
>>>>>> + unsigned int flags;
>>>>>> + enum xfs_delattr_state dela_state;
>>>>>> +};
>>>>>> +
>>>>>> +/*
>>>>>> * Structure to ease passing around component names.
>>>>>> */
>>>>>> typedef struct xfs_da_args {
>>>>>> + struct xfs_delattr_context dac; /* context used for delay attr ops */
>>>>>> struct xfs_da_geometry *geo; /* da block geometry */
>>>>>> struct xfs_name name; /* name, length and argument flags*/
>>>>>> uint8_t filetype; /* filetype of inode for directories */
>>>>>> diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
>>>>>> index 1887605..9a649d1 100644
>>>>>> --- a/fs/xfs/scrub/common.c
>>>>>> +++ b/fs/xfs/scrub/common.c
>>>>>> @@ -24,6 +24,8 @@
>>>>>> #include "xfs_rmap_btree.h"
>>>>>> #include "xfs_log.h"
>>>>>> #include "xfs_trans_priv.h"
>>>>>> +#include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_reflink.h"
>>>>>> #include "scrub/scrub.h"
>>>>>> diff --git a/fs/xfs/xfs_acl.c b/fs/xfs/xfs_acl.c
>>>>>> index 42ac847..d65e6d8 100644
>>>>>> --- a/fs/xfs/xfs_acl.c
>>>>>> +++ b/fs/xfs/xfs_acl.c
>>>>>> @@ -10,6 +10,8 @@
>>>>>> #include "xfs_trans_resv.h"
>>>>>> #include "xfs_mount.h"
>>>>>> #include "xfs_inode.h"
>>>>>> +#include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_trace.h"
>>>>>> #include "xfs_error.h"
>>>>>> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
>>>>>> index d37743b..881b9a4 100644
>>>>>> --- a/fs/xfs/xfs_attr_list.c
>>>>>> +++ b/fs/xfs/xfs_attr_list.c
>>>>>> @@ -12,6 +12,7 @@
>>>>>> #include "xfs_trans_resv.h"
>>>>>> #include "xfs_mount.h"
>>>>>> #include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_inode.h"
>>>>>> #include "xfs_trans.h"
>>>>>> #include "xfs_bmap.h"
>>>>>> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
>>>>>> index 28c07c9..7c1d9da 100644
>>>>>> --- a/fs/xfs/xfs_ioctl.c
>>>>>> +++ b/fs/xfs/xfs_ioctl.c
>>>>>> @@ -15,6 +15,8 @@
>>>>>> #include "xfs_iwalk.h"
>>>>>> #include "xfs_itable.h"
>>>>>> #include "xfs_error.h"
>>>>>> +#include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_bmap.h"
>>>>>> #include "xfs_bmap_util.h"
>>>>>> diff --git a/fs/xfs/xfs_ioctl32.c b/fs/xfs/xfs_ioctl32.c
>>>>>> index 769581a..d504f8f 100644
>>>>>> --- a/fs/xfs/xfs_ioctl32.c
>>>>>> +++ b/fs/xfs/xfs_ioctl32.c
>>>>>> @@ -17,6 +17,8 @@
>>>>>> #include "xfs_itable.h"
>>>>>> #include "xfs_fsops.h"
>>>>>> #include "xfs_rtalloc.h"
>>>>>> +#include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_ioctl.h"
>>>>>> #include "xfs_ioctl32.h"
>>>>>> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
>>>>>> index e85bbf5..a2d299f 100644
>>>>>> --- a/fs/xfs/xfs_iops.c
>>>>>> +++ b/fs/xfs/xfs_iops.c
>>>>>> @@ -13,6 +13,8 @@
>>>>>> #include "xfs_inode.h"
>>>>>> #include "xfs_acl.h"
>>>>>> #include "xfs_quota.h"
>>>>>> +#include "xfs_da_format.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_trans.h"
>>>>>> #include "xfs_trace.h"
>>>>>> diff --git a/fs/xfs/xfs_xattr.c b/fs/xfs/xfs_xattr.c
>>>>>> index 74133a5..d8dc72d 100644
>>>>>> --- a/fs/xfs/xfs_xattr.c
>>>>>> +++ b/fs/xfs/xfs_xattr.c
>>>>>> @@ -10,6 +10,7 @@
>>>>>> #include "xfs_log_format.h"
>>>>>> #include "xfs_da_format.h"
>>>>>> #include "xfs_inode.h"
>>>>>> +#include "xfs_da_btree.h"
>>>>>> #include "xfs_attr.h"
>>>>>> #include "xfs_acl.h"
>>>>>> --
>>>>>> 2.7.4
>>>>>>
>>>>>
>>>>
>>>
>>
>