Re: [PATCH] Fix bug in mm/thrash.c function grab_swap_token()

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

 



On 5/10/07, Mika Kukkonen <[email protected]> wrote:
Following bug was uncovered by compiling with '-W' flag:

  CC      mm/thrash.o
mm/thrash.c: In function 'grab_swap_token':
mm/thrash.c:52: warning: comparison of unsigned expression < 0 is always false

Field token_priority is unsigned, so decrementing first and the checking
is not a good plan. Attached patch (compile tested) reverses the order.

Heh, yeah. Actually, decrementing first and then checking < 0 to clamp
to zero is _never_ a good plan. If the original author thought he was
optimizing speed that way, he was clearly mistaken.

Btw, I'm not sure if likely() makes any sense in this new situation.

Right, if(likely(...)) without an else wouldn't make much sense ...

Signed-off-by: Mika Kukkonen <[email protected]>

diff --git a/mm/thrash.c b/mm/thrash.c
index 9ef9071..c4c5205 100644
--- a/mm/thrash.c
+++ b/mm/thrash.c
@@ -48,9 +48,8 @@ void grab_swap_token(void)
                if (current_interval < current->mm->last_interval)
                        current->mm->token_priority++;
                else {
-                       current->mm->token_priority--;
-                       if (unlikely(current->mm->token_priority < 0))
-                               current->mm->token_priority = 0;
+                       if (likely(current->mm->token_priority > 0))
+                               current->mm->token_priority--;
                }
                /* Check if we deserve the token */
                if (current->mm->token_priority >

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux