On 8/31/21 8:47 PM, Dave Chinner wrote:
On Tue, Aug 24, 2021 at 03:44:29PM -0700, Allison Henderson wrote:
From: Allison Collins <allison.henderson@xxxxxxxxxx>
These routines set up and queue a new deferred attribute operations.
These functions are meant to be called by any routine needing to
initiate a deferred attribute operation as opposed to the existing
inline operations. New helper function xfs_attr_item_init also added.
Finally enable delayed attributes in xfs_attr_set and xfs_attr_remove.
Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
.....
+STATIC int
+xfs_attr_item_init(
+ struct xfs_da_args *args,
+ unsigned int op_flags, /* op flag (set or remove) */
+ struct xfs_attr_item **attr) /* new xfs_attr_item */
+{
+
+ struct xfs_attr_item *new;
+
+ new = kmem_zalloc(sizeof(struct xfs_attr_item), KM_NOFS);
In transaction context here so we don't need KM_NOFS.
ok, will remove
+ new->xattri_op_flags = op_flags;
+ new->xattri_dac.da_args = args;
+
+ *attr = new;
+ return 0;
+}
Why doesn't this just return the object or NULL on allocation
failure? What other error could it ever return?
I had adopted this function signature just to be consistent with other
*_item_init routines at the time. Mostly just trying to be uniform,
though they may have changed since. This patch spent some time on the
back burner while we were more focused on the state machine refactoring.
+
+/* Sets an attribute for an inode as a deferred operation */
+int
+xfs_attr_set_deferred(
+ struct xfs_da_args *args)
+{
+ struct xfs_attr_item *new;
+ int error = 0;
+
+ error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET, &new);
+ if (error)
+ return error;
i.e.
attri = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_SET);
if (!attri)
return -ENOMEM;
+
+ xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new->xattri_list);
+
+ return 0;
+}
+
+/* Removes an attribute for an inode as a deferred operation */
+int
+xfs_attr_remove_deferred(
+ struct xfs_da_args *args)
+{
+
+ struct xfs_attr_item *new;
+ int error;
+
+ error = xfs_attr_item_init(args, XFS_ATTR_OP_FLAGS_REMOVE, &new);
+ if (error)
+ return error;
+
+ xfs_defer_add(args->trans, XFS_DEFER_OPS_TYPE_ATTR, &new->xattri_list);
+
+ return 0;
+}
We really should not use "new" as a variable name. As a general
rule, the common pattern set by this file is that xfs_attri_item
objects in a function are named "attri". Just because it's newly
allocated doesn't mean we should use a different convention for
naming xfs_attri_item objects...
Ok, I had seen the pattern around and reused it. Will change to attri
Allison
Cheers,
Dave.
