Re: [PATCH 03/17] xfs: use xfs_attr_defer_parent for calling xfs_attr_set on pptrs

Darrick J. Wong
2024-04-16T18:41:11Z

On Tue, Apr 16, 2024 at 09:28:09AM -0700, Christoph Hellwig wrote:
> On Tue, Apr 16, 2024 at 09:05:55AM -0700, Darrick J. Wong wrote:
> > I prefer to keep the pptr version separate so that we can assert that
> > the args contents for parent pointers is really correct.
>
> We could do that with a merged version just as easily.

Eh, ok.

> > Looking at
> > xfs_attr_defer_parent again, it also ought to be checking that
> > args->valuelen == sizeof(xfs_getparents_rec);
>
> Yes.

The patch "xfs: create attr log item opcodes and formats for parent
pointers" now contains:

void
xfs_attr_defer_add(
struct xfs_da_args *args,
enum xfs_attr_defer_op op)
{
struct xfs_attr_intent *new;
unsigned int log_op = 0;
bool is_pptr = args->attr_filter & XFS_ATTR_PARENT;

if (is_pptr) {
ASSERT(xfs_has_parent(args->dp->i_mount));
ASSERT((args->attr_filter & ~XFS_ATTR_PARENT) != 0);
ASSERT(args->op_flags & XFS_DA_OP_LOGGED);
ASSERT(args->valuelen == sizeof(struct xfs_parent_rec));
}

new = kmem_cache_zalloc(xfs_attr_intent_cache,
GFP_NOFS | __GFP_NOFAIL);
new->xattri_da_args = args;

/* Compute log operation from the higher level op and namespace. */
switch (op) {
case XFS_ATTR_DEFER_SET:
if (is_pptr)
log_op = XFS_ATTRI_OP_FLAGS_PPTR_SET;
else
log_op = XFS_ATTRI_OP_FLAGS_SET;
break;
case XFS_ATTR_DEFER_REPLACE:
if (is_pptr)
log_op = XFS_ATTRI_OP_FLAGS_PPTR_REPLACE;
else
log_op = XFS_ATTRI_OP_FLAGS_REPLACE;
break;
case XFS_ATTR_DEFER_REMOVE:
if (is_pptr)
log_op = XFS_ATTRI_OP_FLAGS_PPTR_REMOVE;
else
log_op = XFS_ATTRI_OP_FLAGS_REMOVE;
break;
default:
ASSERT(0);
break;
}
new->xattri_op_flags = log_op;

/* Set up initial attr operation state. */
switch (log_op) {
case XFS_ATTRI_OP_FLAGS_PPTR_SET:
case XFS_ATTRI_OP_FLAGS_SET:
new->xattri_dela_state = xfs_attr_init_add_state(args);
break;
case XFS_ATTRI_OP_FLAGS_PPTR_REPLACE:
ASSERT(args->new_valuelen == args->valuelen);
new->xattri_dela_state = xfs_attr_init_replace_state(args);
break;
case XFS_ATTRI_OP_FLAGS_REPLACE:
new->xattri_dela_state = xfs_attr_init_replace_state(args);
break;
case XFS_ATTRI_OP_FLAGS_PPTR_REMOVE:
case XFS_ATTRI_OP_FLAGS_REMOVE:
new->xattri_dela_state = xfs_attr_init_remove_state(args);
break;
}

xfs_defer_add(args->trans, &new->xattri_list, &xfs_attr_defer_type);
trace_xfs_attr_defer_add(new->xattri_dela_state, args->dp);
}

and then we can drop this patch.

--D