Re: [PATCH v7 02/23] xfs: Add helper function xfs_attr_try_sf_addname

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

 



On 06/09/2018 11:58 PM, Amir Goldstein wrote:
On Sun, Jun 10, 2018 at 8:04 AM, Allison Henderson
<allison.henderson@xxxxxxxxxx> wrote:
This patch adds a subroutine xfs_attr_try_sf_addname
used by xfs_attr_set.  This subrotine will attempt to
add the attribute name specified in args in shortform,
as well and perform error handling previously done in
xfs_attr_set.

This patch helps to pre-simplify xfs_attr_set for reviewing
purposes and reduce indentation.  New function will be added
in the next patch.

Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
---
  fs/xfs/libxfs/xfs_attr.c | 64 +++++++++++++++++++++++++++++++-----------------
  1 file changed, 41 insertions(+), 23 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_attr.c b/fs/xfs/libxfs/xfs_attr.c
index ce4a34a..1524197 100644
--- a/fs/xfs/libxfs/xfs_attr.c
+++ b/fs/xfs/libxfs/xfs_attr.c
@@ -203,6 +203,44 @@ xfs_attr_calc_size(
         return nblks;
  }

+STATIC int
+xfs_attr_try_sf_addname(
+       struct xfs_inode        *dp,
+       struct xfs_da_args      *args,
+       int                     flags)
+{
+
+       struct xfs_mount        *mp = dp->i_mount;
+       int                     error;
+       int                     err2;
+
+       error = xfs_attr_shortform_addname(args);
+       if (error != -ENOSPC) {

Not sure it made sense before, but in the context of this helper
nesting most of the code path for the likely case is weird.
Please return error if (error == -ENOSPC)
Sure, maybe I could do that change in the next patch. I'm trying to avoid moving code and then changing it at the same time because I think it makes the reviews a little harder on folks. Will fix though. Thx!

Allison


+               /*
+                * Commit the shortform mods, and we're done.
+                * NOTE: this is also the error path
+                * (EEXIST, etc).
+                */
+               ASSERT(args->trans != NULL);
+
+               /*
+                * If this is a synchronous mount, make sure
+                * that the transaction goes to disk before
+                * returning to the user.
+                */
+               if (mp->m_flags & XFS_MOUNT_WSYNC)
+                       xfs_trans_set_sync(args->trans);
+
+               if (!error && (flags & ATTR_KERNOTIME) == 0) {
+                       xfs_trans_ichgtime(args->trans, dp,
+                                       XFS_ICHGTIME_CHG);
+               }
+               err2 = xfs_trans_commit(args->trans);
+               error = error ? error : err2;
+       }
+       return error;
+}
+
  int
  xfs_attr_set(
         struct xfs_inode        *dp,
@@ -218,7 +256,7 @@ xfs_attr_set(
         struct xfs_trans_res    tres;
         xfs_fsblock_t           firstblock;
         int                     rsvd = (flags & ATTR_ROOT) != 0;
-       int                     error, err2, local;
+       int                     error, local;

         XFS_STATS_INC(mp, xs_attr_set);

@@ -297,30 +335,10 @@ xfs_attr_set(
                  * Try to add the attr to the attribute list in
                  * the inode.
                  */
-               error = xfs_attr_shortform_addname(&args);
+               error = xfs_attr_try_sf_addname(dp, &args, flags);
                 if (error != -ENOSPC) {
-                       /*
-                        * Commit the shortform mods, and we're done.
-                        * NOTE: this is also the error path (EEXIST, etc).
-                        */
-                       ASSERT(args.trans != NULL);
-
-                       /*
-                        * If this is a synchronous mount, make sure that
-                        * the transaction goes to disk before returning
-                        * to the user.
-                        */
-                       if (mp->m_flags & XFS_MOUNT_WSYNC)
-                               xfs_trans_set_sync(args.trans);
-
-                       if (!error && (flags & ATTR_KERNOTIME) == 0) {
-                               xfs_trans_ichgtime(args.trans, dp,
-                                                       XFS_ICHGTIME_CHG);
-                       }
-                       err2 = xfs_trans_commit(args.trans);
                         xfs_iunlock(dp, XFS_ILOCK_EXCL);
-
-                       return error ? error : err2;
+                       return error;
                 }

                 /*
--
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=LHZQ8fHvy6wDKXGTWcm97burZH5sQKHRDMaY1UthQxc&m=xVunh0uk8Pktex1LHFbHolstfv3eqBBkWt5RkoG7tPA&s=eaQpPcYIgNL0hFyZdNc-dOXhzugeysrtVYd50uOAO1Q&e=
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  https://urldefense.proofpoint.com/v2/url?u=http-3A__vger.kernel.org_majordomo-2Dinfo.html&d=DwIBaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=LHZQ8fHvy6wDKXGTWcm97burZH5sQKHRDMaY1UthQxc&m=xVunh0uk8Pktex1LHFbHolstfv3eqBBkWt5RkoG7tPA&s=eaQpPcYIgNL0hFyZdNc-dOXhzugeysrtVYd50uOAO1Q&e=

--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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