Page MenuHomeFreeBSD

timerfd: Fix interval callout scheduling
ClosedPublic

Authored by jfree on Tue, Mar 10, 12:51 AM.
Tags
None
Referenced Files
F150715244: D55790.id173514.diff
Fri, Apr 3, 1:56 PM
Unknown Object (File)
Wed, Apr 1, 4:35 PM
Unknown Object (File)
Wed, Mar 25, 9:06 AM
Unknown Object (File)
Tue, Mar 24, 9:09 AM
Unknown Object (File)
Tue, Mar 24, 6:08 AM
Unknown Object (File)
Sun, Mar 22, 6:29 AM
Unknown Object (File)
Fri, Mar 20, 9:23 PM
Unknown Object (File)
Fri, Mar 20, 5:09 AM
Subscribers

Details

Summary

When a timerfd interval callout misses its scheduled activation time, a
differential is calculated based on the actual activation time and the
scheduled activation time. This differential is divided by the timerfd's
interval time and the quotient is added to the timerfd's counter.

Before this change, the next callout was scheduled to activate at:
scheduled activation time + timerfd interval.

This change fixes the scheduling of the next callout to activate at:
actual activation time + timerfd interval - remainder.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tests/sys/kern/timerfd.c
955

Checking that errno == 0 everywhere seems odd to me. errno is generally only meaningful after a library function fails. I especially don't see why the ATF runtime should guarantee that errno == 0 at the beginning of the test.

tests/sys/kern/timerfd.c
955

Agreed - I was following the style of the rest of the test cases. I'll update this new test case to remove this excessive errno checking. Do you think I should update D55789 to remove the excessive checking throughout the entire file?

Rewrite timerfd__missed_events test to remove excessive errno
checking.

tests/sys/kern/timerfd.c
971

I'm not certain, but this might need to be >= 1001 instead, to accommodate test environments with high load that might induce pauses in test cases.

Also I wonder whether 1001 is really correct here, vs. 1000? I can see why this returns 1001 from the implementation, but is it right?

tests/sys/kern/timerfd.c
971

It is funny you mention this. I spent way too much time trying to cleanly implement a way to generate 1000 timeouts exclusively for expirations set in the past. After finally settling on a solution, I ran the test case on Linux and their implementation generates 1001 timeouts.

I guess it kind of makes sense... The initial expiration is always counted as +1 (even if it is in the past) then every expiration until the present is an additional +1.

I will make the test case >= 1001 here to account for high load.

tests/sys/kern/timerfd.c
971

I'm not certain, but this might need to be >= 1001 instead, to accommodate test environments with high load that might induce pauses in test cases.

On second thought, the system would need to freeze for over a second in order for this count to be >= 1001. I think keeping it at 1001 is reasonable in this case.

markj added inline comments.
tests/sys/kern/timerfd.c
971

That's not too out of the ordinary in oversubscribed, virtualized CI environments. Jenkins might complain about it.

This revision is now accepted and ready to land.Sat, Mar 14, 1:06 AM
This revision was automatically updated to reflect the committed changes.