Page MenuHomeFreeBSD

timerfd: Fix interval callout scheduling
AcceptedPublic

Authored by jfree on Tue, Mar 10, 12:51 AM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Mar 11, 5:50 AM
Unknown Object (File)
Wed, Mar 11, 4:33 AM
Subscribers

Details

Reviewers
markj
imp
arrowd
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 Skipped
Unit
Tests Skipped
Build Status
Buildable 71316
Build 68199: arc lint + arc unit

Event Timeline

tests/sys/kern/timerfd.c
987

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
987

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
1003

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
1003

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
1003

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
1003

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