Comment 51 for bug 595555

Revision history for this message
In , christopher.halse.rogers (christopher.halse.rogers-redhat-bugs) wrote :

My commit messages might have mislead you. There are two commits there, neither of which change the definition of a +ve or -ve transition trigger.

The first one adjusts the bounds that the sync implementation is looking for. Currently, if you've got a -ve transition threshold of 60, the sync code will ignore any event over 60 *and not update the current count*. This was the main problem, which the first patch fixes by ensuring the bracket values where the sync code will respond to events are enough to get the current value *past* the thresholds.

This doesn't change the meaning of either a positive or negative transition, just ensures that they'll actually trigger in appropriate circumstances.

The second patch, requiring a wakeup in 1ms if we exactly hit the threshold, is really more for completeness than anything. The server tends to get woken up multiple times in a millisecond anyway.

(In reply to comment #38)
...snip...
> > The SYNC extension defines a negative transition as triggered from a
> > counter strictly *above* the threshold to ≤ the threshold
>
> If this is the case, why do you need to change anything? The current
> implementation (before your change) correctly implemented this
> definition. But could you point me to where this is defined? [1]
> seems to be the standard about SYNC ext, but it leaves the exact
> definition of +/- trans. to the implementation. [2] has concrete
> definition, which is same as Def.1. But [2] is a Programmer's Guide,
> not a doc defining SYNC ext.
>
> > I think the changes in [Comment #27] ... are wrong.
>
> Yup, it committed the crime of going against a Programmer's Guide,
> which is carved in the stone. Reminded me of the guy who was in jail
> for two years, because he failed to put a US. Federal-required label
> on his UPS package.

I'm sorry if I caused offence. I didn't mean to denigrate your work; indeed, your analysis of the problem was very helpful. I meant that I think changing the meaning of the triggers is not the correct way to solve the problem. Other applications using SYNC objects other than IDLETIME might be broken by changing the meaning.