Re: [PATCH] net/ipv4/*, net/core/neighbour.c jiffies cleanup

[Andreas Dilger <adilger@turbolabs.com>]

On Nov 07, 2001  16:09 -0800, David S. Miller wrote:
>    From: Tim Schmielau <tim@physik3.uni-rostock.de>
> 
>    jiffies cleanup patch of the day follows. Mostly boring changes of jiffies
>    comparisons to use time_{before,after} in order to handle jiffies
>    wraparound correctly.
> 
> These cases handle wraparound correctly!!!!
> 
> Please stop sending these changes, start thinking about what the
> code is doing.
> 
> It is comparing a "DIFFERRENCE" not raw jiffy values with each other.
> It works just fine.

No, only a limited number of them cast to a signed value, which means
that a large number of them get the comparison wrong in the case of
jiffies wrap (where the difference is a large unsigned value, and not
a small negative number).


This is not just idle change.  Tim has problems when jiffies is
initialized to a pre-wrap value at boot, and changing everything to
use time_{before,after} is the only easy way to audit all of the code
(and know that it is done).

As I sent to Alan privately (and he agreed), there are three reasons to
change this code (even if it is correct) to using time_{before,after}:

1) because it is non-obvious what "correct" is when dealing with jiffies wrap
   (some of the changes that Alan previously complained about as being already
   correct were in fact broken, and if _he_ can't get it right, who can?)
2) so that people see it more and are more likely to get it correct, instead
   of always adding in code that only breaks after 497 days of uptime
3) to isolate code from any changes if jiffies moves to a 64-bit value (where
   casts to "(long)" may not be appropriate anymore)

Cheers, Andreas
--
Andreas Dilger
http://sourceforge.net/projects/ext2resize/
http://www-mddsp.enel.ucalgary.ca/People/adilger/