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

[Andreas Dilger]

On Nov 08, 2001  08:55 -0800, Krishna Kumar wrote:
> > > In short: It is wrong to do
> > >
> > >          if (jiffies <= start+HZ)
> > >
> > > and it is _right_ to do
> > >
> > >          if (jiffies - start <= HZ)
> >
> > Actually this last part is wrong, isn't it ? jiffies <= start + HZ is
> > also a correct way to do it, since start+HZ will overflow to the current
> > value of jiffies when HZ time elapses. So the above two statements are
> > IDENTICAL.
> 
> I am sorry, but I still don't see the difference. I wrote a small program
> with different cases, but the values still come same irrespective of the
> input arguments to the checks. Could you tell under what conditions the
> checks wuold fail ? The 2's complement  works the same for addition and
> subtraction. I have included the test program below.

There are several cases where jiffies comparisons can be incorrect:

	unsigned long start = jiffies;		/* say 0xfffffff8 */
	unsigned long delta = 16;
	unsigned long end = jiffies + delta;	/* wraps to 0x00000008 */

a)	while (jiffies < end) { do something }

If jiffies is near wrap when calculating end, end will wrap and your loop
will never be executed, because 0xfffffff9 is greater than 0x00000008.

b)	while (jiffies - end < 0) { do something }

Fails because both jiffies and end are unsigned, and the difference can
never be negative.

c)	while ((long)jiffies - (long)end < 0) { do something }

Correct.  Hmm, this is just like

	while(time_before(jiffies, end)) { do something }

d) 	while (jiffies - start < delta) { do something }

This will also work because of modulo arithmetic, as long as we know in
advance that "start" is always less than "jiffies".

e)	if (jiffies > end) { fail because of timeout }

Incorrect for the same reason as (a) above, 0xfffffff9 > 0x00000008, where
we really want to wait until 0x00000009 to timeout.

f)	if (jiffies - end > 0) { fail because of timeout }

Fails for the same reason as (b) - both jiffies and end are unsigned, and
the difference can never be negative.

g)	if ((long)jiffies - (long)end > 0) { fail because of timeout }

Correct, which is just like:

	if (time_after(jiffies, end)) { fail because of timeout }

h)	if (jiffies - start > delta) { fail because of timeout }

Correct because of modulo arithmetic, as long as we know in advance that
"start" is less than "jiffies".

So it appears there are lots of ways to get it wrong that appear to be
correct.  This is why I'm pushing for the use of the macros, so that
there is no question about whether the comparison is right or wrong.

This is just the same as Linus' pedantic min() and max() macros - sure
people can get it right by themselves, but sometimes they get it wrong,
and it is easier to just make sure they don't get it wrong.

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