Fix 'negative objects to delete' warning
authorBrian Behlendorf <behlendorf1@llnl.gov>
Mon, 9 May 2011 19:18:46 +0000 (12:18 -0700)
committerBrian Behlendorf <behlendorf1@llnl.gov>
Wed, 18 May 2011 17:29:22 +0000 (10:29 -0700)
Normally when the arc_shrinker_func() function is called the return
value should be:

   >=0 - To indicate the number of freeable objects in the cache, or
   -1  - To indicate this cache should be skipped

However, when the shrinker callback is called with 'nr_to_scan' equal
to zero.  The caller simply wants the number of freeable objects in
the cache and we must never return -1.  This patch reorders the
first two conditionals in arc_shrinker_func() to ensure this behavior.

This patch also now explictly casts arc_size and arc_c_min to signed
int64_t types so MAX(x, 0) works as expected.  As unsigned types
we would never see an negative value which defeated the purpose of
the MAX() lower bound and broke the shrinker logic.

Finally, when nr_to_scan is non-zero we explictly prevent all reclaim
below arc_c_min.  This is done to prevent the Linux page cache from
completely crowding out the ARC.  This limit is tunable and some
experimentation is likely going to be required to set it exactly right.
For now we're sticking with the OpenSolaris defaults.

Closes #218
Closes #243

module/zfs/arc.c

index f1f972b..eb5304a 100644 (file)
@@ -2196,15 +2196,20 @@ SPL_SHRINKER_CALLBACK_PROTO(arc_shrinker_func, cb, nr_to_scan, gfp_mask)
        arc_reclaim_strategy_t strategy;
        int arc_reclaim;
 
-       /* Not allowed to perform filesystem reclaim */
-       if (!(gfp_mask & __GFP_FS))
-               return (-1);
-
        /* Return number of reclaimable pages based on arc_shrink_shift */
-       arc_reclaim = btop((arc_size - arc_c_min)) >> arc_shrink_shift;
+       arc_reclaim = MAX(btop(((int64_t)arc_size - (int64_t)arc_c_min))
+           >> arc_shrink_shift, 0);
        if (nr_to_scan == 0)
                return (arc_reclaim);
 
+       /* Prevent reclaim below arc_c_min */
+       if (arc_reclaim <= 0)
+               return (-1);
+
+       /* Not allowed to perform filesystem reclaim */
+       if (!(gfp_mask & __GFP_FS))
+               return (-1);
+
        /* Reclaim in progress */
        if (mutex_tryenter(&arc_reclaim_thr_lock) == 0)
                return (-1);
@@ -2218,7 +2223,8 @@ SPL_SHRINKER_CALLBACK_PROTO(arc_shrinker_func, cb, nr_to_scan, gfp_mask)
        }
 
        arc_kmem_reap_now(strategy);
-       arc_reclaim = btop((arc_size - arc_c_min)) >> arc_shrink_shift;
+       arc_reclaim = MAX(btop(((int64_t)arc_size - (int64_t)arc_c_min))
+           >> arc_shrink_shift, 0);
        mutex_exit(&arc_reclaim_thr_lock);
 
        return (arc_reclaim);