Page MenuHomeFreeBSD

Add a test for cancelling an active AIO request on a socket.
ClosedPublic

Authored by jhb on Dec 3 2015, 11:08 PM.

Details

Summary

Add a test for cancelling an active AIO request on a socket.

The older AIO code awakened all pending AIO requests on a socket
when any data arrived. This could result in AIO daemons blocking on
an empty socket buffer. These requests could not be cancelled
which led to a deadlock during process exit. This test reproduces
this case. The newer AIO code is able to cancel the pending AIO
request correctly.

Test Plan
  • Run the test on an old kernel. Not only does the process hang and refuse to exit, but the kernel hangs on reboot.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb retitled this revision from to Queue socket requests one at a time instead of a broadcast..
jhb updated this object.
jhb edited the test plan for this revision. (Show Details)
jhb added reviewers: glebius, ngie.
jhb added a subscriber: np.

One question I have is if we want to check in a test that has such a negative effect on broken systems.

The test portion looks ok, minus the minor nit in the Makefile.

tests/sys/aio/Makefile
7 ↗(On Diff #10713)

Could you please put this on a separate line?

I need to sort this list too >_>...

In D4363#91952, @jhb wrote:

One question I have is if we want to check in a test that has such a negative effect on broken systems.

The test's good. It might be a good idea to key whether or not it should be skipped based on FreeBSD_version though.

In D4363#93334, @ngie wrote:
In D4363#91952, @jhb wrote:

One question I have is if we want to check in a test that has such a negative effect on broken systems.

The test's good. It might be a good idea to key whether or not it should be skipped based on FreeBSD_version though.

I've finally gotten back to this. Should that be a runtime check of the current kernel's version? If so, do we have an existing ATF helper for that ala ATF_REQUIRE_MODULE. I'm thinking something like:

ATF_REQUIRE_OSRELDATE(1100000) and having it skip if the kern.osreldate is older?

jhb edited edge metadata.

Rework this change to only include the test. The FIFO behavior was included
in the AIO rework committed recently.

  • Add a test for the hang when two reads are queued.
  • Update comment for what happened when I ran the test.
  • Fix copyright.
  • Implement ATF_REQUIRE_OSRELDATE() and PLAIN_REQUIRE_OSRELDATE().
  • Use ATF_REQUIRE_OSRELDATE().
  • Compile.
jhb retitled this revision from Queue socket requests one at a time instead of a broadcast. to Add a test for cancelling an active AIO request on a socket..Mar 19 2016, 12:33 AM
jhb updated this object.
jhb updated this object.
  • Style and misc fixes.
  • Rebase
  • Move test over into aio_test.
tests/freebsd_test_suite/macros.h
55–67 ↗(On Diff #14756)

This seems like a bit of an unnecessary hoop to jump through, especially because it can be changed by jail(8). What I was proposing was something more along the lines of the following in the test(s):

#if __FreeBSD_version < X`
    atf_tc_skip("not running test on system because it's too old");
#endif

We could make an official macro for it that doesn't require the kern.osrelease poking though...

tests/freebsd_test_suite/macros.h
55–67 ↗(On Diff #14756)

If you think a compile time test (vs run-time) is fine I'm ok with that.

  • Drop OSRELDATE macros and test __FreeBSD_version at compile time.
This revision was automatically updated to reflect the committed changes.