Messages in this thread Patch in this message | | | Date | Thu, 10 May 2007 20:56:20 +0530 | From | "Satyam Sharma" <> | Subject | Re: [PATCH] Fix bug in mm/thrash.c function grab_swap_token() |
| |
On 5/10/07, Mika Kukkonen <mikukkon@miku.homelinux.net> 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 <mikukkon@iki.fi> > > 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 majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |