xfs: remove infinite loop when reserving free block pool
authorDarrick J. Wong <djwong@kernel.org>
Fri, 11 Mar 2022 18:56:01 +0000 (10:56 -0800)
committerDarrick J. Wong <djwong@kernel.org>
Mon, 28 Mar 2022 15:38:50 +0000 (08:38 -0700)
Infinite loops in kernel code are scary.  Calls to xfs_reserve_blocks
should be rare (people should just use the defaults!) so we really don't
need to try so hard.  Simplify the logic here by removing the infinite
loop.

Cc: Brian Foster <bfoster@redhat.com>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
fs/xfs/xfs_fsops.c

index 710e857..3c6d9d6 100644 (file)
@@ -430,46 +430,36 @@ xfs_reserve_blocks(
         * If the request is larger than the current reservation, reserve the
         * blocks before we update the reserve counters. Sample m_fdblocks and
         * perform a partial reservation if the request exceeds free space.
+        *
+        * The code below estimates how many blocks it can request from
+        * fdblocks to stash in the reserve pool.  This is a classic TOCTOU
+        * race since fdblocks updates are not always coordinated via
+        * m_sb_lock.
         */
-       error = -ENOSPC;
-       do {
-               free = percpu_counter_sum(&mp->m_fdblocks) -
+       free = percpu_counter_sum(&mp->m_fdblocks) -
                                                xfs_fdblocks_unavailable(mp);
-               if (free <= 0)
-                       break;
-
-               delta = request - mp->m_resblks;
-               lcounter = free - delta;
-               if (lcounter < 0)
-                       /* We can't satisfy the request, just get what we can */
-                       fdblks_delta = free;
-               else
-                       fdblks_delta = delta;
-
+       delta = request - mp->m_resblks;
+       if (delta > 0 && free > 0) {
                /*
                 * We'll either succeed in getting space from the free block
-                * count or we'll get an ENOSPC. If we get a ENOSPC, it means
-                * things changed while we were calculating fdblks_delta and so
-                * we should try again to see if there is anything left to
-                * reserve.
-                *
-                * Don't set the reserved flag here - we don't want to reserve
-                * the extra reserve blocks from the reserve.....
+                * count or we'll get an ENOSPC.  Don't set the reserved flag
+                * here - we don't want to reserve the extra reserve blocks
+                * from the reserve.
                 */
+               fdblks_delta = min(free, delta);
                spin_unlock(&mp->m_sb_lock);
                error = xfs_mod_fdblocks(mp, -fdblks_delta, 0);
                spin_lock(&mp->m_sb_lock);
-       } while (error == -ENOSPC);
 
-       /*
-        * Update the reserve counters if blocks have been successfully
-        * allocated.
-        */
-       if (!error && fdblks_delta) {
-               mp->m_resblks += fdblks_delta;
-               mp->m_resblks_avail += fdblks_delta;
+               /*
+                * Update the reserve counters if blocks have been successfully
+                * allocated.
+                */
+               if (!error) {
+                       mp->m_resblks += fdblks_delta;
+                       mp->m_resblks_avail += fdblks_delta;
+               }
        }
-
 out:
        if (outval) {
                outval->resblks = mp->m_resblks;