Dear James,
I would be much more enlightened if your patch had included the code for
time_after() and time_before().
Also, a refreshing of what gcc does with x>y, (i.e. does it use an
overflow detecting instruction or just a subtract). Assuming the former
x>y is different from x-y>0, the latter working as long as the true
difference between x & y is less than MAXPOSINT. From this prespective
I don't understand how something like "if (jiffies - reset_start_time >
2*HZ/100)" is a problem unless you are really concerned about times that
are 248+ days apart.
But, then again, the missing time_after/before code might be the key.
George
James Manning wrote:
>
> This is a partial (and alpha) patch to cleaning up jiffies wraps problem
> areas. I wanted to get your feedback before completing the patch as
> it will be sizable. Specifically, the most common (by far) construct
> (after patching) seems to be if(time_after(jiffies,start+timeout)) for
> checking timeout conditions, so I wanted to see if you wanted another
> abstraction macro to make the code a little easier to read. Also,
> there are some places where conditions are expressed as:
>
> if(SHAPERCB(skb)->shapeclock - jiffies > SHAPER_LATENCY) {
>
> Aside from the sign-check-only nature of possible optimization with
> time_before/time_after, I wanted to make sure that each check of this kind
> of code (jiffies checking in general) code boiled down to one register
> infrequently re-calculated getting checked against a single memory access.
>
> Specifically, "if (jiffies - timeout > 5*HZ)" will result (aside from
> what a gcc -O "should" do) in each pass doing a load from memory, a sub,
> then a cmp against a non-zero (invalidating possible optimization for
> sign-check-only). even just going to "jiffies > timeout + 5*HZ" allows
> the check to now be one volatile element (which we compare immediately
> after load, instead of comparing an ALU result that could be one off
> since jiffies changed underneath us). Also, the "timeout + 5*HZ" part
> is a pure constant combined with a non-volatile variable that doesn't
> change nearly as often as jiffies does.
>
> For normal timeout-ish code, the if(time_after(jiffies,start+timeout))
> works great, but for the above SHAPER_LATENCY check I wanted your
> opinion on the right method to deal with this. Specifically, a move to
> time_after(SHAPERCB(skb)->shapeclock - jiffies, SHAPER_LATENCY) may appear
> valid on first glance, but I believe that it's invalid. jiffies wraps and
> if ->shapeclock is near the wrapping point, the first arg to time_after
> will be large in magnitude even if the reality is a latency of only a few.
>
> James
>
> ------------------------------------------------------------------------
>
> jw.diffName: jw.diff
> Type: Plain Text (text/plain)
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/
This archive was generated by hypermail 2b29 : Mon May 15 2000 - 21:00:16 EST