Commit e7f358de authored by Dave Chinner's avatar Dave Chinner Committed by Dave Chinner
Browse files

xfs: use XFS_DA_OP flags in deferred attr ops



We currently store the high level attr operation in
args->attr_flags. This field contains what the VFS is telling us to
do, but don't necessarily match what we are doing in the low level
modification state machine. e.g. XATTR_REPLACE implies both
XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME because it is doing both a
remove and adding a new attr.

However, deep in the individual state machine operations, we check
errors against this high level VFS op flags, not the low level
XFS_DA_OP flags. Indeed, we don't even have a low level flag for
a REMOVE operation, so the only way we know we are doing a remove
is the complete absence of XATTR_REPLACE, XATTR_CREATE,
XFS_DA_OP_ADDNAME and XFS_DA_OP_RENAME. And because there are other
flags in these fields, this is a pain to check if we need to.

As the XFS_DA_OP flags are only needed once the deferred operations
are set up, set these flags appropriately when we set the initial
operation state. We also introduce a XFS_DA_OP_REMOVE flag to make
it easy to know that we are doing a remove operation.

With these, we can remove the use of XATTR_REPLACE and XATTR_CREATE
in low level lookup operations, and manipulate the low level flags
according to the low level context that is operating. e.g. log
recovery does not have a VFS xattr operation state to copy into
args->attr_flags, and the low level state machine ops we do for
recovery do not match the high level VFS operations that were in
progress when the system failed...

Signed-off-by: default avatarDave Chinner <dchinner@redhat.com>
Reviewed-by: default avatarDarrick J. Wong <djwong@kernel.org>
Reviewed-by: default avatarAllison Henderson <allison.henderson@oracle.com>
Signed-off-by: default avatarDave Chinner <david@fromorbit.com>
parent 59782a23
Loading
Loading
Loading
Loading
+74 −62
Original line number Original line Diff line number Diff line
@@ -466,7 +466,7 @@ xfs_attr_leaf_addname(
	 */
	 */
	if (args->rmtblkno)
	if (args->rmtblkno)
		attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT;
		attr->xattri_dela_state = XFS_DAS_LEAF_SET_RMT;
	else if (args->op_flags & XFS_DA_OP_RENAME)
	else if (args->op_flags & XFS_DA_OP_REPLACE)
		xfs_attr_dela_state_set_replace(attr, XFS_DAS_LEAF_REPLACE);
		xfs_attr_dela_state_set_replace(attr, XFS_DAS_LEAF_REPLACE);
	else
	else
		attr->xattri_dela_state = XFS_DAS_DONE;
		attr->xattri_dela_state = XFS_DAS_DONE;
@@ -511,7 +511,7 @@ xfs_attr_node_addname(


	if (args->rmtblkno)
	if (args->rmtblkno)
		attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT;
		attr->xattri_dela_state = XFS_DAS_NODE_SET_RMT;
	else if (args->op_flags & XFS_DA_OP_RENAME)
	else if (args->op_flags & XFS_DA_OP_REPLACE)
		xfs_attr_dela_state_set_replace(attr, XFS_DAS_NODE_REPLACE);
		xfs_attr_dela_state_set_replace(attr, XFS_DAS_NODE_REPLACE);
	else
	else
		attr->xattri_dela_state = XFS_DAS_DONE;
		attr->xattri_dela_state = XFS_DAS_DONE;
@@ -547,7 +547,7 @@ xfs_attr_rmtval_alloc(
		return error;
		return error;


	/* If this is not a rename, clear the incomplete flag and we're done. */
	/* If this is not a rename, clear the incomplete flag and we're done. */
	if (!(args->op_flags & XFS_DA_OP_RENAME)) {
	if (!(args->op_flags & XFS_DA_OP_REPLACE)) {
		error = xfs_attr3_leaf_clearflag(args);
		error = xfs_attr3_leaf_clearflag(args);
		attr->xattri_dela_state = XFS_DAS_DONE;
		attr->xattri_dela_state = XFS_DAS_DONE;
	} else {
	} else {
@@ -966,8 +966,6 @@ xfs_attr_set(


	if (args->value) {
	if (args->value) {
		XFS_STATS_INC(mp, xs_attr_set);
		XFS_STATS_INC(mp, xs_attr_set);

		args->op_flags |= XFS_DA_OP_ADDNAME;
		args->total = xfs_attr_calc_size(args, &local);
		args->total = xfs_attr_calc_size(args, &local);


		/*
		/*
@@ -1125,28 +1123,41 @@ static inline int xfs_attr_sf_totsize(struct xfs_inode *dp)
 * Add a name to the shortform attribute list structure
 * Add a name to the shortform attribute list structure
 * This is the external routine.
 * This is the external routine.
 */
 */
STATIC int
static int
xfs_attr_shortform_addname(xfs_da_args_t *args)
xfs_attr_shortform_addname(
	struct xfs_da_args	*args)
{
{
	int newsize, forkoff, retval;
	int			newsize, forkoff;
	int			error;


	trace_xfs_attr_sf_addname(args);
	trace_xfs_attr_sf_addname(args);


	retval = xfs_attr_shortform_lookup(args);
	error = xfs_attr_shortform_lookup(args);
	if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
	switch (error) {
		return retval;
	case -ENOATTR:
	if (retval == -EEXIST) {
		if (args->op_flags & XFS_DA_OP_REPLACE)
		if (args->attr_flags & XATTR_CREATE)
			return error;
			return retval;
		break;
		retval = xfs_attr_sf_removename(args);
	case -EEXIST:
		if (retval)
		if (!(args->op_flags & XFS_DA_OP_REPLACE))
			return retval;
			return error;

		error = xfs_attr_sf_removename(args);
		if (error)
			return error;

		/*
		/*
		 * Since we have removed the old attr, clear ATTR_REPLACE so
		 * Since we have removed the old attr, clear XFS_DA_OP_REPLACE
		 * that the leaf format add routine won't trip over the attr
		 * so that the new attr doesn't fit in shortform format, the
		 * not being around.
		 * leaf format add routine won't trip over the attr not being
		 * around.
		 */
		 */
		args->attr_flags &= ~XATTR_REPLACE;
		args->op_flags &= ~XFS_DA_OP_REPLACE;
		break;
	case 0:
		break;
	default:
		return error;
	}
	}


	if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
	if (args->namelen >= XFS_ATTR_SF_ENTSIZE_MAX ||
@@ -1169,8 +1180,8 @@ xfs_attr_shortform_addname(xfs_da_args_t *args)
 * External routines when attribute list is one block
 * External routines when attribute list is one block
 *========================================================================*/
 *========================================================================*/


/* Store info about a remote block */
/* Save the current remote block info and clear the current pointers. */
STATIC void
static void
xfs_attr_save_rmt_blk(
xfs_attr_save_rmt_blk(
	struct xfs_da_args	*args)
	struct xfs_da_args	*args)
{
{
@@ -1179,10 +1190,13 @@ xfs_attr_save_rmt_blk(
	args->rmtblkno2 = args->rmtblkno;
	args->rmtblkno2 = args->rmtblkno;
	args->rmtblkcnt2 = args->rmtblkcnt;
	args->rmtblkcnt2 = args->rmtblkcnt;
	args->rmtvaluelen2 = args->rmtvaluelen;
	args->rmtvaluelen2 = args->rmtvaluelen;
	args->rmtblkno = 0;
	args->rmtblkcnt = 0;
	args->rmtvaluelen = 0;
}
}


/* Set stored info about a remote block */
/* Set stored info about a remote block */
STATIC void
static void
xfs_attr_restore_rmt_blk(
xfs_attr_restore_rmt_blk(
	struct xfs_da_args	*args)
	struct xfs_da_args	*args)
{
{
@@ -1228,28 +1242,27 @@ xfs_attr_leaf_try_add(
	 * Look up the xattr name to set the insertion point for the new xattr.
	 * Look up the xattr name to set the insertion point for the new xattr.
	 */
	 */
	error = xfs_attr3_leaf_lookup_int(bp, args);
	error = xfs_attr3_leaf_lookup_int(bp, args);
	if (error != -ENOATTR && error != -EEXIST)
	switch (error) {
		goto out_brelse;
	case -ENOATTR:
	if (error == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
		if (args->op_flags & XFS_DA_OP_REPLACE)
			goto out_brelse;
			goto out_brelse;
	if (error == -EEXIST) {
		break;
		if (args->attr_flags & XATTR_CREATE)
	case -EEXIST:
		if (!(args->op_flags & XFS_DA_OP_REPLACE))
			goto out_brelse;
			goto out_brelse;


		trace_xfs_attr_leaf_replace(args);
		trace_xfs_attr_leaf_replace(args);

		/* save the attribute state for later removal*/
		args->op_flags |= XFS_DA_OP_RENAME;	/* an atomic rename */
		xfs_attr_save_rmt_blk(args);

		/*
		/*
		 * clear the remote attr state now that it is saved so that the
		 * Save the existing remote attr state so that the current
		 * values reflect the state of the attribute we are about to
		 * values reflect the state of the new attribute we are about to
		 * add, not the attribute we just found and will remove later.
		 * add, not the attribute we just found and will remove later.
		 */
		 */
		args->rmtblkno = 0;
		xfs_attr_save_rmt_blk(args);
		args->rmtblkcnt = 0;
		break;
		args->rmtvaluelen = 0;
	case 0:
		break;
	default:
		goto out_brelse;
	}
	}


	return xfs_attr3_leaf_add(bp, args);
	return xfs_attr3_leaf_add(bp, args);
@@ -1391,43 +1404,42 @@ xfs_attr_node_addname_find_attr(
	 struct xfs_attr_item	*attr)
	 struct xfs_attr_item	*attr)
{
{
	struct xfs_da_args	*args = attr->xattri_da_args;
	struct xfs_da_args	*args = attr->xattri_da_args;
	int				retval;
	int			error;


	/*
	/*
	 * Search to see if name already exists, and get back a pointer
	 * Search to see if name already exists, and get back a pointer
	 * to where it should go.
	 * to where it should go.
	 */
	 */
	retval = xfs_attr_node_hasname(args, &attr->xattri_da_state);
	error = xfs_attr_node_hasname(args, &attr->xattri_da_state);
	if (retval != -ENOATTR && retval != -EEXIST)
	switch (error) {
		goto error;
	case -ENOATTR:

		if (args->op_flags & XFS_DA_OP_REPLACE)
	if (retval == -ENOATTR && (args->attr_flags & XATTR_REPLACE))
			goto error;
			goto error;
	if (retval == -EEXIST) {
		break;
		if (args->attr_flags & XATTR_CREATE)
	case -EEXIST:
		if (!(args->op_flags & XFS_DA_OP_REPLACE))
			goto error;
			goto error;


		trace_xfs_attr_node_replace(args);

		/* save the attribute state for later removal*/
		args->op_flags |= XFS_DA_OP_RENAME;	/* atomic rename op */
		xfs_attr_save_rmt_blk(args);


		trace_xfs_attr_node_replace(args);
		/*
		/*
		 * clear the remote attr state now that it is saved so that the
		 * Save the existing remote attr state so that the current
		 * values reflect the state of the attribute we are about to
		 * values reflect the state of the new attribute we are about to
		 * add, not the attribute we just found and will remove later.
		 * add, not the attribute we just found and will remove later.
		 */
		 */
		args->rmtblkno = 0;
		xfs_attr_save_rmt_blk(args);
		args->rmtblkcnt = 0;
		break;
		args->rmtvaluelen = 0;
	case 0:
		break;
	default:
		goto error;
	}
	}


	return 0;
	return 0;
error:
error:
	if (attr->xattri_da_state)
	if (attr->xattri_da_state)
		xfs_da_state_free(attr->xattri_da_state);
		xfs_da_state_free(attr->xattri_da_state);
	return retval;
	return error;
}
}


/*
/*
+4 −1
Original line number Original line Diff line number Diff line
@@ -584,7 +584,6 @@ xfs_attr_is_shortform(
static inline enum xfs_delattr_state
static inline enum xfs_delattr_state
xfs_attr_init_add_state(struct xfs_da_args *args)
xfs_attr_init_add_state(struct xfs_da_args *args)
{
{

	/*
	/*
	 * When called from the completion of a attr remove to determine the
	 * When called from the completion of a attr remove to determine the
	 * next state, the attribute fork may be null. This can occur only occur
	 * next state, the attribute fork may be null. This can occur only occur
@@ -595,6 +594,8 @@ xfs_attr_init_add_state(struct xfs_da_args *args)
	 */
	 */
	if (!args->dp->i_afp)
	if (!args->dp->i_afp)
		return XFS_DAS_DONE;
		return XFS_DAS_DONE;

	args->op_flags |= XFS_DA_OP_ADDNAME;
	if (xfs_attr_is_shortform(args->dp))
	if (xfs_attr_is_shortform(args->dp))
		return XFS_DAS_SF_ADD;
		return XFS_DAS_SF_ADD;
	if (xfs_attr_is_leaf(args->dp))
	if (xfs_attr_is_leaf(args->dp))
@@ -605,6 +606,7 @@ xfs_attr_init_add_state(struct xfs_da_args *args)
static inline enum xfs_delattr_state
static inline enum xfs_delattr_state
xfs_attr_init_remove_state(struct xfs_da_args *args)
xfs_attr_init_remove_state(struct xfs_da_args *args)
{
{
	args->op_flags |= XFS_DA_OP_REMOVE;
	if (xfs_attr_is_shortform(args->dp))
	if (xfs_attr_is_shortform(args->dp))
		return XFS_DAS_SF_REMOVE;
		return XFS_DAS_SF_REMOVE;
	if (xfs_attr_is_leaf(args->dp))
	if (xfs_attr_is_leaf(args->dp))
@@ -615,6 +617,7 @@ xfs_attr_init_remove_state(struct xfs_da_args *args)
static inline enum xfs_delattr_state
static inline enum xfs_delattr_state
xfs_attr_init_replace_state(struct xfs_da_args *args)
xfs_attr_init_replace_state(struct xfs_da_args *args)
{
{
	args->op_flags |= XFS_DA_OP_ADDNAME | XFS_DA_OP_REPLACE;
	return xfs_attr_init_add_state(args);
	return xfs_attr_init_add_state(args);
}
}


+1 −1
Original line number Original line Diff line number Diff line
@@ -1492,7 +1492,7 @@ xfs_attr3_leaf_add_work(
	entry->flags = args->attr_filter;
	entry->flags = args->attr_filter;
	if (tmp)
	if (tmp)
		entry->flags |= XFS_ATTR_LOCAL;
		entry->flags |= XFS_ATTR_LOCAL;
	if (args->op_flags & XFS_DA_OP_RENAME) {
	if (args->op_flags & XFS_DA_OP_REPLACE) {
		if (!xfs_has_larp(mp))
		if (!xfs_has_larp(mp))
			entry->flags |= XFS_ATTR_INCOMPLETE;
			entry->flags |= XFS_ATTR_INCOMPLETE;
		if ((args->blkno2 == args->blkno) &&
		if ((args->blkno2 == args->blkno) &&
+5 −3
Original line number Original line Diff line number Diff line
@@ -85,19 +85,21 @@ typedef struct xfs_da_args {
 * Operation flags:
 * Operation flags:
 */
 */
#define XFS_DA_OP_JUSTCHECK	(1u << 0) /* check for ok with no space */
#define XFS_DA_OP_JUSTCHECK	(1u << 0) /* check for ok with no space */
#define XFS_DA_OP_RENAME	(1u << 1) /* this is an atomic rename op */
#define XFS_DA_OP_REPLACE	(1u << 1) /* this is an atomic replace op */
#define XFS_DA_OP_ADDNAME	(1u << 2) /* this is an add operation */
#define XFS_DA_OP_ADDNAME	(1u << 2) /* this is an add operation */
#define XFS_DA_OP_OKNOENT	(1u << 3) /* lookup op, ENOENT ok, else die */
#define XFS_DA_OP_OKNOENT	(1u << 3) /* lookup op, ENOENT ok, else die */
#define XFS_DA_OP_CILOOKUP	(1u << 4) /* lookup returns CI name if found */
#define XFS_DA_OP_CILOOKUP	(1u << 4) /* lookup returns CI name if found */
#define XFS_DA_OP_NOTIME	(1u << 5) /* don't update inode timestamps */
#define XFS_DA_OP_NOTIME	(1u << 5) /* don't update inode timestamps */
#define XFS_DA_OP_REMOVE	(1u << 6) /* this is a remove operation */


#define XFS_DA_OP_FLAGS \
#define XFS_DA_OP_FLAGS \
	{ XFS_DA_OP_JUSTCHECK,	"JUSTCHECK" }, \
	{ XFS_DA_OP_JUSTCHECK,	"JUSTCHECK" }, \
	{ XFS_DA_OP_RENAME,	"RENAME" }, \
	{ XFS_DA_OP_REPLACE,	"REPLACE" }, \
	{ XFS_DA_OP_ADDNAME,	"ADDNAME" }, \
	{ XFS_DA_OP_ADDNAME,	"ADDNAME" }, \
	{ XFS_DA_OP_OKNOENT,	"OKNOENT" }, \
	{ XFS_DA_OP_OKNOENT,	"OKNOENT" }, \
	{ XFS_DA_OP_CILOOKUP,	"CILOOKUP" }, \
	{ XFS_DA_OP_CILOOKUP,	"CILOOKUP" }, \
	{ XFS_DA_OP_NOTIME,	"NOTIME" }
	{ XFS_DA_OP_NOTIME,	"NOTIME" }, \
	{ XFS_DA_OP_REMOVE,	"REMOVE" }


/*
/*
 * Storage for holding state during Btree searches and split/join ops.
 * Storage for holding state during Btree searches and split/join ops.