Skip to content

Commit 411b434

Browse files
dgchinnerdchinner
dgchinner
authored and
dchinner
committed
xfs: XFS_DAS_LEAF_REPLACE state only needed if !LARP
We can skip the REPLACE state when LARP is enabled, but that means the XFS_DAS_FLIP_LFLAG state is now poorly named - it indicates something that has been done rather than what the state is going to do. Rename it to "REMOVE_OLD" to indicate that we are now going to perform removal of the old attr. Signed-off-by: Dave Chinner <dchinner@redhat.com> Reviewed-by: Allison Henderson<allison.henderson@oracle.com> Reviewed-by: Darrick J. Wong <djwong@kernel.org> Signed-off-by: Dave Chinner <david@fromorbit.com>
1 parent 7d03533 commit 411b434

3 files changed

Lines changed: 75 additions & 54 deletions

File tree

‎fs/xfs/libxfs/xfs_attr.c‎

Lines changed: 51 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -295,6 +295,26 @@ xfs_attr_sf_addname(
295295
return error;
296296
}
297297

298+
/*
299+
* When we bump the state to REPLACE, we may actually need to skip over the
300+
* state. When LARP mode is enabled, we don't need to run the atomic flags flip,
301+
* so we skip straight over the REPLACE state and go on to REMOVE_OLD.
302+
*/
303+
static void
304+
xfs_attr_dela_state_set_replace(
305+
struct xfs_attr_item *attr,
306+
enum xfs_delattr_state replace)
307+
{
308+
struct xfs_da_args *args = attr->xattri_da_args;
309+
310+
ASSERT(replace == XFS_DAS_LEAF_REPLACE ||
311+
replace == XFS_DAS_NODE_REPLACE);
312+
313+
attr->xattri_dela_state = replace;
314+
if (xfs_has_larp(args->dp->i_mount))
315+
attr->xattri_dela_state++;
316+
}
317+
298318
static int
299319
xfs_attr_leaf_addname(
300320
struct xfs_attr_item *attr)
@@ -337,7 +357,7 @@ xfs_attr_leaf_addname(
337357
attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT;
338358
error = -EAGAIN;
339359
} else if (args->op_flags & XFS_DA_OP_RENAME) {
340-
attr->xattri_dela_state = XFS_DAS_LEAF_REPLACE;
360+
xfs_attr_dela_state_set_replace(attr, XFS_DAS_LEAF_REPLACE);
341361
error = -EAGAIN;
342362
} else {
343363
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -368,7 +388,7 @@ xfs_attr_node_addname(
368388
attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT;
369389
error = -EAGAIN;
370390
} else if (args->op_flags & XFS_DA_OP_RENAME) {
371-
attr->xattri_dela_state = XFS_DAS_NODE_REPLACE;
391+
xfs_attr_dela_state_set_replace(attr, XFS_DAS_NODE_REPLACE);
372392
error = -EAGAIN;
373393
} else {
374394
attr->xattri_dela_state = XFS_DAS_DONE;
@@ -395,8 +415,11 @@ xfs_attr_rmtval_alloc(
395415
error = xfs_attr_rmtval_set_blk(attr);
396416
if (error)
397417
return error;
398-
error = -EAGAIN;
399-
goto out;
418+
/* Roll the transaction only if there is more to allocate. */
419+
if (attr->xattri_blkcnt > 0) {
420+
error = -EAGAIN;
421+
goto out;
422+
}
400423
}
401424

402425
error = xfs_attr_rmtval_set_value(args);
@@ -407,6 +430,13 @@ xfs_attr_rmtval_alloc(
407430
if (!(args->op_flags & XFS_DA_OP_RENAME)) {
408431
error = xfs_attr3_leaf_clearflag(args);
409432
attr->xattri_dela_state = XFS_DAS_DONE;
433+
} else {
434+
/*
435+
* We are running a REPLACE operation, so we need to bump the
436+
* state to the step in that operation.
437+
*/
438+
attr->xattri_dela_state++;
439+
xfs_attr_dela_state_set_replace(attr, attr->xattri_dela_state);
410440
}
411441
out:
412442
trace_xfs_attr_rmtval_alloc(attr->xattri_dela_state, args->dp);
@@ -428,7 +458,6 @@ xfs_attr_set_iter(
428458
struct xfs_inode *dp = args->dp;
429459
struct xfs_buf *bp = NULL;
430460
int forkoff, error = 0;
431-
struct xfs_mount *mp = args->dp->i_mount;
432461

433462
/* State machine switch */
434463
next_state:
@@ -458,37 +487,29 @@ xfs_attr_set_iter(
458487
return error;
459488
if (attr->xattri_dela_state == XFS_DAS_DONE)
460489
break;
461-
attr->xattri_dela_state++;
462-
fallthrough;
490+
goto next_state;
463491

464492
case XFS_DAS_LEAF_REPLACE:
465493
case XFS_DAS_NODE_REPLACE:
466494
/*
467-
* If this is an atomic rename operation, we must "flip" the
468-
* incomplete flags on the "new" and "old" attribute/value pairs
469-
* so that one disappears and one appears atomically. Then we
470-
* must remove the "old" attribute/value pair.
471-
*
472-
* In a separate transaction, set the incomplete flag on the
473-
* "old" attr and clear the incomplete flag on the "new" attr.
495+
* We must "flip" the incomplete flags on the "new" and "old"
496+
* attribute/value pairs so that one disappears and one appears
497+
* atomically. Then we must remove the "old" attribute/value
498+
* pair.
474499
*/
475-
if (!xfs_has_larp(mp)) {
476-
error = xfs_attr3_leaf_flipflags(args);
477-
if (error)
478-
return error;
479-
/*
480-
* Commit the flag value change and start the next trans
481-
* in series at FLIP_FLAG.
482-
*/
483-
error = -EAGAIN;
484-
attr->xattri_dela_state++;
485-
break;
486-
}
487-
500+
error = xfs_attr3_leaf_flipflags(args);
501+
if (error)
502+
return error;
503+
/*
504+
* Commit the flag value change and start the next trans
505+
* in series at REMOVE_OLD.
506+
*/
507+
error = -EAGAIN;
488508
attr->xattri_dela_state++;
489-
fallthrough;
490-
case XFS_DAS_FLIP_LFLAG:
491-
case XFS_DAS_FLIP_NFLAG:
509+
break;
510+
511+
case XFS_DAS_LEAF_REMOVE_OLD:
512+
case XFS_DAS_NODE_REMOVE_OLD:
492513
/*
493514
* Dismantle the "old" attribute/value pair by removing a
494515
* "remote" value (if it exists).

‎fs/xfs/libxfs/xfs_attr.h‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -455,42 +455,42 @@ enum xfs_delattr_state {
455455
XFS_DAS_LEAF_SET_RMT, /* set a remote xattr from a leaf */
456456
XFS_DAS_LEAF_ALLOC_RMT, /* We are allocating remote blocks */
457457
XFS_DAS_LEAF_REPLACE, /* Perform replace ops on a leaf */
458-
XFS_DAS_FLIP_LFLAG, /* Flipped leaf INCOMPLETE attr flag */
458+
XFS_DAS_LEAF_REMOVE_OLD, /* Start removing old attr from leaf */
459459
XFS_DAS_RM_LBLK, /* A rename is removing leaf blocks */
460460
XFS_DAS_RD_LEAF, /* Read in the new leaf */
461461

462462
/* Node state set sequence, must match leaf state above */
463463
XFS_DAS_NODE_SET_RMT, /* set a remote xattr from a node */
464464
XFS_DAS_NODE_ALLOC_RMT, /* We are allocating remote blocks */
465465
XFS_DAS_NODE_REPLACE, /* Perform replace ops on a node */
466-
XFS_DAS_FLIP_NFLAG, /* Flipped node INCOMPLETE attr flag */
466+
XFS_DAS_NODE_REMOVE_OLD, /* Start removing old attr from node */
467467
XFS_DAS_RM_NBLK, /* A rename is removing node blocks */
468468
XFS_DAS_CLR_FLAG, /* Clear incomplete flag */
469469

470470
XFS_DAS_DONE, /* finished operation */
471471
};
472472

473473
#define XFS_DAS_STRINGS \
474-
{ XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \
475-
{ XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \
476-
{ XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \
477-
{ XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \
478-
{ XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \
479-
{ XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \
480-
{ XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \
481-
{ XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \
482-
{ XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \
483-
{ XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \
484-
{ XFS_DAS_FLIP_LFLAG, "XFS_DAS_FLIP_LFLAG" }, \
485-
{ XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \
486-
{ XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \
487-
{ XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \
488-
{ XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \
489-
{ XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \
490-
{ XFS_DAS_FLIP_NFLAG, "XFS_DAS_FLIP_NFLAG" }, \
491-
{ XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \
492-
{ XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \
493-
{ XFS_DAS_DONE, "XFS_DAS_DONE" }
474+
{ XFS_DAS_UNINIT, "XFS_DAS_UNINIT" }, \
475+
{ XFS_DAS_SF_ADD, "XFS_DAS_SF_ADD" }, \
476+
{ XFS_DAS_LEAF_ADD, "XFS_DAS_LEAF_ADD" }, \
477+
{ XFS_DAS_NODE_ADD, "XFS_DAS_NODE_ADD" }, \
478+
{ XFS_DAS_RMTBLK, "XFS_DAS_RMTBLK" }, \
479+
{ XFS_DAS_RM_NAME, "XFS_DAS_RM_NAME" }, \
480+
{ XFS_DAS_RM_SHRINK, "XFS_DAS_RM_SHRINK" }, \
481+
{ XFS_DAS_LEAF_SET_RMT, "XFS_DAS_LEAF_SET_RMT" }, \
482+
{ XFS_DAS_LEAF_ALLOC_RMT, "XFS_DAS_LEAF_ALLOC_RMT" }, \
483+
{ XFS_DAS_LEAF_REPLACE, "XFS_DAS_LEAF_REPLACE" }, \
484+
{ XFS_DAS_LEAF_REMOVE_OLD, "XFS_DAS_LEAF_REMOVE_OLD" }, \
485+
{ XFS_DAS_RM_LBLK, "XFS_DAS_RM_LBLK" }, \
486+
{ XFS_DAS_RD_LEAF, "XFS_DAS_RD_LEAF" }, \
487+
{ XFS_DAS_NODE_SET_RMT, "XFS_DAS_NODE_SET_RMT" }, \
488+
{ XFS_DAS_NODE_ALLOC_RMT, "XFS_DAS_NODE_ALLOC_RMT" }, \
489+
{ XFS_DAS_NODE_REPLACE, "XFS_DAS_NODE_REPLACE" }, \
490+
{ XFS_DAS_NODE_REMOVE_OLD, "XFS_DAS_NODE_REMOVE_OLD" }, \
491+
{ XFS_DAS_RM_NBLK, "XFS_DAS_RM_NBLK" }, \
492+
{ XFS_DAS_CLR_FLAG, "XFS_DAS_CLR_FLAG" }, \
493+
{ XFS_DAS_DONE, "XFS_DAS_DONE" }
494494

495495
/*
496496
* Defines for xfs_attr_item.xattri_flags

‎fs/xfs/xfs_trace.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4139,13 +4139,13 @@ TRACE_DEFINE_ENUM(XFS_DAS_RM_SHRINK);
41394139
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_SET_RMT);
41404140
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_ALLOC_RMT);
41414141
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REPLACE);
4142-
TRACE_DEFINE_ENUM(XFS_DAS_FLIP_LFLAG);
4142+
TRACE_DEFINE_ENUM(XFS_DAS_LEAF_REMOVE_OLD);
41434143
TRACE_DEFINE_ENUM(XFS_DAS_RM_LBLK);
41444144
TRACE_DEFINE_ENUM(XFS_DAS_RD_LEAF);
41454145
TRACE_DEFINE_ENUM(XFS_DAS_NODE_SET_RMT);
41464146
TRACE_DEFINE_ENUM(XFS_DAS_NODE_ALLOC_RMT);
41474147
TRACE_DEFINE_ENUM(XFS_DAS_NODE_REPLACE);
4148-
TRACE_DEFINE_ENUM(XFS_DAS_FLIP_NFLAG);
4148+
TRACE_DEFINE_ENUM(XFS_DAS_NODE_REMOVE_OLD);
41494149
TRACE_DEFINE_ENUM(XFS_DAS_RM_NBLK);
41504150
TRACE_DEFINE_ENUM(XFS_DAS_CLR_FLAG);
41514151

0 commit comments

Comments
 (0)