Re: [PATCH] 2.5.13 IDE 50

[Tim Schmielau]

> - Fix wrong usage of time_after in ide.c. This should cure the drive 
> seek
>    timeout problems some people where expierencing. This was clarified 
> to me by
>    Bartek, who apparently checked whatever the actual code is consistent 
> with the comments in front of it. Thank you Bartlomiej Zolnierkiewicz.
> 
>    I think now that we should have time_past(xxx) in <linux/timer.h>.

What would you suppose time_past(xxx) to do?

I agree this calls for some action to prevent confusion in the future.
However, I'm not sure how a new macro could help here.

Andreas Dilger once did a patch to clarify the documentation of
time_[before,after] a bit.
(http://www.lib.uaa.alaska.edu/linux-kernel/archive/2001-Week-45/0075.html)

> @@ -1258,7 +1172,7 @@
>  
>                         /* This device still wants to remain idle.
>                          */
> -                       if (drive->sleep && time_after(jiffies, drive->sleep))
> +                       if (drive->sleep && time_after(drive->sleep, jiffies))
>                                 continue;
>  
>                         /* Take this device, if there is no device choosen thus
> 

I think there is an implicit notational convention to have the volatile 
argument, i.e. jiffies, first. This would express that the condition 
evaluates as true *before* some fixed point in time:

   if (drive->sleep && time_before(jiffies, drive->sleep))

Maybe sticking to this convention would suffice to keep the 
semantics of the condition obvious.

Tim