Re: [PATCH v8 13/20] xfs: Add helpers xfs_attr_is_shortform and xfs_attr_set_shortform

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/7/20 2:53 PM, Allison Collins wrote:


On 4/7/20 8:23 AM, Brian Foster wrote:
On Fri, Apr 03, 2020 at 03:12:22PM -0700, Allison Collins wrote:
In this patch, we hoist code from xfs_attr_set_args into two new helpers
xfs_attr_is_shortform and xfs_attr_set_shortform.  These two will help
to simplify xfs_attr_set_args when we get into delayed attrs later.

Signed-off-by: Allison Collins <allison.henderson@xxxxxxxxxx>
---
  fs/xfs/libxfs/xfs_attr.c | 107 +++++++++++++++++++++++++++++++----------------
  1 file changed, 72 insertions(+), 35 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index 4225a94..ba26ffe 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -204,6 +204,66 @@ xfs_attr_try_sf_addname(
  }
  /*
+ * Check to see if the attr should be upgraded from non-existent or shortform to
+ * single-leaf-block attribute list.
+ */
+static inline bool
+xfs_attr_is_shortform(
+    struct xfs_inode    *ip)
+{
+    return ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL ||
+          (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
+          ip->i_d.di_anextents == 0);

Logic should be indented similar to the original:

    return ip->i_d.di_aformat == XFS_DINODE_FMT_LOCAL ||
           (ip->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
        ip->i_d.di_anextents == 0);
Did you really mean to have the last line offset like this? On second pass, it doesnt look similar to the original, and looks more like it may have been a typo in the review. Just trying to avoid more cycles on spacing goofs. Thx!

Allison


Otherwise looks good:

Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
Alrighty, will fix.  Thanks!

Allison


+}
+
+/*
+ * Attempts to set an attr in shortform, or converts the tree to leaf form if + * there is not enough room.  If the attr is set, the transaction is committed
+ * and set to NULL.
+ */
+STATIC int
+xfs_attr_set_shortform(
+    struct xfs_da_args    *args,
+    struct xfs_buf        **leaf_bp)
+{
+    struct xfs_inode    *dp = args->dp;
+    int            error, error2 = 0;
+
+    /*
+     * Try to add the attr to the attribute list in the inode.
+     */
+    error = xfs_attr_try_sf_addname(dp, args);
+    if (error != -ENOSPC) {
+        error2 = xfs_trans_commit(args->trans);
+        args->trans = NULL;
+        return error ? error : error2;
+    }
+    /*
+     * It won't fit in the shortform, transform to a leaf block.  GROT:
+     * another possible req'mt for a double-split btree op.
+     */
+    error = xfs_attr_shortform_to_leaf(args, leaf_bp);
+    if (error)
+        return error;
+
+    /*
+     * Prevent the leaf buffer from being unlocked so that a concurrent AIL +     * push cannot grab the half-baked leaf buffer and run into problems +     * with the write verifier. Once we're done rolling the transaction we
+     * can release the hold and add the attr to the leaf.
+     */
+    xfs_trans_bhold(args->trans, *leaf_bp);
+    error = xfs_defer_finish(&args->trans);
+    xfs_trans_bhold_release(args->trans, *leaf_bp);
+    if (error) {
+        xfs_trans_brelse(args->trans, *leaf_bp);
+        return error;
+    }
+
+    return 0;
+}
+
+/*
   * Set the attribute specified in @args.
   */
  int
@@ -212,48 +272,25 @@ xfs_attr_set_args(
  {
      struct xfs_inode    *dp = args->dp;
      struct xfs_buf          *leaf_bp = NULL;
-    int            error, error2 = 0;
+    int            error = 0;
      /*
-     * If the attribute list is non-existent or a shortform list,
-     * upgrade it to a single-leaf-block attribute list.
+     * If the attribute list is already in leaf format, jump straight to +     * leaf handling.  Otherwise, try to add the attribute to the shortform +     * list; if there's no room then convert the list to leaf format and try
+     * again.
       */
-    if (dp->i_d.di_aformat == XFS_DINODE_FMT_LOCAL ||
-        (dp->i_d.di_aformat == XFS_DINODE_FMT_EXTENTS &&
-         dp->i_d.di_anextents == 0)) {
-
-        /*
-         * Try to add the attr to the attribute list in the inode.
-         */
-        error = xfs_attr_try_sf_addname(dp, args);
-        if (error != -ENOSPC) {
-            error2 = xfs_trans_commit(args->trans);
-            args->trans = NULL;
-            return error ? error : error2;
-        }
-
-        /*
-         * It won't fit in the shortform, transform to a leaf block.
-         * GROT: another possible req'mt for a double-split btree op.
-         */
-        error = xfs_attr_shortform_to_leaf(args, &leaf_bp);
-        if (error)
-            return error;
+    if (xfs_attr_is_shortform(dp)) {
          /*
-         * Prevent the leaf buffer from being unlocked so that a
-         * concurrent AIL push cannot grab the half-baked leaf
-         * buffer and run into problems with the write verifier.
-         * Once we're done rolling the transaction we can release
-         * the hold and add the attr to the leaf.
+         * If the attr was successfully set in shortform, the
+         * transaction is committed and set to NULL.  Otherwise, is it
+         * converted from shortform to leaf, and the transaction is
+         * retained.
           */
-        xfs_trans_bhold(args->trans, leaf_bp);
-        error = xfs_defer_finish(&args->trans);
-        xfs_trans_bhold_release(args->trans, leaf_bp);
-        if (error) {
-            xfs_trans_brelse(args->trans, leaf_bp);
+        error = xfs_attr_set_shortform(args, &leaf_bp);
+        if (error || !args->trans)
              return error;
-        }
      }
      if (xfs_bmap_one_block(dp, XFS_ATTR_FORK)) {
--
2.7.4





[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux