xfs
[Top] [All Lists]

[PATCH 1/5] percpu_counter: fix test in __percpu_counter_add_unless_lt()

To: XFS Mailing List <xfs@xxxxxxxxxxx>
Subject: [PATCH 1/5] percpu_counter: fix test in __percpu_counter_add_unless_lt()
From: Alex Elder <aelder@xxxxxxx>
Date: Wed, 22 Dec 2010 21:56:15 -0600
Reply-to: aelder@xxxxxxx
In __percpu_counter_add_unless_lt(), there is a test to see if
a call to __percpu_counter_add() can be made, knowing the result
will not be less than the given threshhold and the called function
will do the addition without taking a lock.

Unfortunately, it is using the wrong value in its comparison--the
amount to add is not properly being taken into account.  As a
result, __percpu_counter_add() could end up adding a value when it
should not.  And even if the result will be non-negative, the called
function may take the lock anyway because it does so if the *sum* of
the (percpu) delta count and the given amount (and not just the amount
alone) is greater than the batch size (or less than its negative).

Fix the comparison, and since it __percpu_counter_add() will do
the right thing (and might lock anyway), just call it regardless
of what amount is getting added.

Signed-off-by: Alex Elder <aelder@xxxxxxx>

---
 lib/percpu_counter.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Index: b/lib/percpu_counter.c
===================================================================
--- a/lib/percpu_counter.c
+++ b/lib/percpu_counter.c
@@ -239,10 +239,10 @@ int __percpu_counter_add_unless_lt(struc
                goto out;
 
        /*
-        * If the counter is over the threshold and the change is less than the
-        * batch size, we might be able to avoid locking.
+        * If the updated counter will be over the threshold we know
+        * we can safely add, and might be able to avoid locking.
         */
-       if (count > threshold + error && abs(amount) < batch) {
+       if (count + amount > threshold + error) {
                __percpu_counter_add(fbc, amount, batch);
                ret = 1;
                goto out;


<Prev in Thread] Current Thread [Next in Thread>