nanosleep(2): Fix bogus incrementing of rmtp by tc_tick_sbt on [EINTR].
ClosedPublic

Authored by bdrewery on Tue, Feb 13, 10:30 PM.

Details

Summary

sbt is the time in the future that the tsleep_sbt() is expected to be completed
at. sbtt is the current time. Depending on the precision with sysctl
kern.timecounter.alloweddeviation the start time may be incremented by
tc_tick_sbt. The same increment is needed for the current time of sbtt before
calculating the difference. The impact of missing this increment is that rmtp
may increase by one tc_tick_sbt on every early [EINTR] return. If the same
struct is passed in for rqtp as rmtp this can result in rqtp effectively
incrementing by tc_tick_sbt and sleeping longer than originally intended .

Sponsored by: Dell EMC
MFC after: 2 weeks

Test Plan

Before:

/usr/tests/lib/libc/sys # /usr/bin/time ./nanosleep_test nanosleep_eintr
nanosleep_test: WARNING: Running test cases outside of kyua(1) is unsupported
nanosleep_test: WARNING: No isolation nor timeout control is being applied; you may get unexpected failures; see atf-test-case(4)
failed: /root/git/freebsd/contrib/netbsd-tests/lib/libc/sys/t_nanosleep.c:231: errno: 4 got_info: 1 ts=5.000000242 should be <= tso=5.000000000

failed: nanosleep(2) handled rtmp incorrectly
        0.05 real         0.00 user         0.00 sys

After:

/usr/tests/lib/libc/sys # /usr/bin/time ./nanosleep_test nanosleep_eintr
nanosleep_test: WARNING: Running test cases outside of kyua(1) is unsupported
nanosleep_test: WARNING: No isolation nor timeout control is being applied; you may get unexpected failures; see atf-test-case(4)
passed
        5.00 real         0.00 user         0.00 sys

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
bdrewery created this revision.Tue, Feb 13, 10:30 PM
bdrewery updated this revision to Diff 39285.Tue, Feb 13, 10:32 PM

Remove unneeded leftover debugging

bdrewery edited the test plan for this revision. (Show Details)Tue, Feb 13, 10:34 PM
kib accepted this revision.Wed, Feb 14, 1:59 PM
This revision is now accepted and ready to land.Wed, Feb 14, 1:59 PM
markj accepted this revision.Wed, Feb 14, 3:00 PM
markj added inline comments.
contrib/netbsd-tests/lib/libc/sys/t_nanosleep.c
215 ↗(On Diff #39285)

I'm not sure that ATF_REQUIRE will DTRT in a child process.

251 ↗(On Diff #39285)

I think it's reasonable to use ATF_REQUIRE to check the return values of these syscalls.

vangyzen accepted this revision.Wed, Feb 14, 3:19 PM
bdrewery added inline comments.Wed, Feb 14, 5:15 PM
contrib/netbsd-tests/lib/libc/sys/t_nanosleep.c
215 ↗(On Diff #39285)

It ends up exiting non-zero which the later wait does pickup on. See my testing example.

251 ↗(On Diff #39285)

Sure

This revision was automatically updated to reflect the committed changes.