xfs: streamline xfs_filestream_pick_ag

Directly return the error from xfs_bmap_longest_free_extent instead
of breaking from the loop and handling it there, and use a done
label to directly jump to the exist when we found a suitable perag
structure to reduce the indentation level and pag/max_pag check
complexity in the tail of the function.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Carlos Maiolino <cem@kernel.org>

diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
index 88bd23ce74cde131bd7d9a8a919441ad2ae73b7e..290ba8887d29e9b73fc535a872ed374b3ee69af8 100644 (file)
--- a/fs/xfs/xfs_filestream.c
+++ b/fs/xfs/xfs_filestream.c
@@ -67,22 +67,28 @@ xfs_filestream_pick_ag(
        xfs_extlen_t            minfree, maxfree = 0;
        xfs_agnumber_t          agno;
        bool                    first_pass = true;
-       int                     err;
 
        /* 2% of an AG's blocks must be free for it to be chosen. */
        minfree = mp->m_sb.sb_agblocks / 50;
 
 restart:
        for_each_perag_wrap(mp, start_agno, agno, pag) {
+               int             err;
+
                trace_xfs_filestream_scan(pag, pino);
+
                *longest = 0;
                err = xfs_bmap_longest_free_extent(pag, NULL, longest);
                if (err) {
-                       if (err != -EAGAIN)
-                               break;
-                       /* Couldn't lock the AGF, skip this AG. */
-                       err = 0;
-                       continue;
+                       if (err == -EAGAIN) {
+                               /* Couldn't lock the AGF, skip this AG. */
+                               err = 0;
+                               continue;
+                       }
+                       xfs_perag_rele(pag);
+                       if (max_pag)
+                               xfs_perag_rele(max_pag);
+                       return err;
                }
 
                /* Keep track of the AG with the most free blocks. */
@@ -107,7 +113,9 @@ restart:
                             !(flags & XFS_PICK_USERDATA) ||
                             (flags & XFS_PICK_LOWSPACE))) {
                                /* Break out, retaining the reference on the AG. */
-                               break;
+                               if (max_pag)
+                                       xfs_perag_rele(max_pag);
+                               goto done;
                        }
                }
 
@@ -115,56 +123,44 @@ restart:
                atomic_dec(&pag->pagf_fstrms);
        }
 
-       if (err) {
-               xfs_perag_rele(pag);
-               if (max_pag)
-                       xfs_perag_rele(max_pag);
-               return err;
+       /*
+        * Allow a second pass to give xfs_bmap_longest_free_extent() another
+        * attempt at locking AGFs that it might have skipped over before we
+        * fail.
+        */
+       if (first_pass) {
+               first_pass = false;
+               goto restart;
        }
 
-       if (!pag) {
-               /*
-                * Allow a second pass to give xfs_bmap_longest_free_extent()
-                * another attempt at locking AGFs that it might have skipped
-                * over before we fail.
-                */
-               if (first_pass) {
-                       first_pass = false;
-                       goto restart;
-               }
-
-               /*
-                * We must be low on data space, so run a final lowspace
-                * optimised selection pass if we haven't already.
-                */
-               if (!(flags & XFS_PICK_LOWSPACE)) {
-                       flags |= XFS_PICK_LOWSPACE;
-                       goto restart;
-               }
-
-               /*
-                * No unassociated AGs are available, so select the AG with the
-                * most free space, regardless of whether it's already in use by
-                * another filestream. It none suit, just use whatever AG we can
-                * grab.
-                */
-               if (!max_pag) {
-                       for_each_perag_wrap(args->mp, 0, start_agno, pag) {
-                               max_pag = pag;
-                               break;
-                       }
+       /*
+        * We must be low on data space, so run a final lowspace optimised
+        * selection pass if we haven't already.
+        */
+       if (!(flags & XFS_PICK_LOWSPACE)) {
+               flags |= XFS_PICK_LOWSPACE;
+               goto restart;
+       }
 
-                       /* Bail if there are no AGs at all to select from. */
-                       if (!max_pag)
-                               return -ENOSPC;
+       /*
+        * No unassociated AGs are available, so select the AG with the most
+        * free space, regardless of whether it's already in use by another
+        * filestream. It none suit, just use whatever AG we can grab.
+        */
+       if (!max_pag) {
+               for_each_perag_wrap(args->mp, 0, start_agno, pag) {
+                       max_pag = pag;
+                       break;
                }
 
-               pag = max_pag;
-               atomic_inc(&pag->pagf_fstrms);
-       } else if (max_pag) {
-               xfs_perag_rele(max_pag);
+               /* Bail if there are no AGs at all to select from. */
+               if (!max_pag)
+                       return -ENOSPC;
        }
 
+       pag = max_pag;
+       atomic_inc(&pag->pagf_fstrms);
+done:
        trace_xfs_filestream_pick(pag, pino);
        args->pag = pag;
        return 0;