Page MenuHomeFreeBSD
Differential D4363

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

Authored by jhb on Dec 3 2015, 11:08 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Aug 24, 12:24 PM
Unknown Object (File)
Thu, Aug 20, 11:45 AM
Unknown Object (File)
Wed, Aug 19, 11:49 AM
Unknown Object (File)
Wed, Aug 19, 11:48 AM
Unknown Object (File)
Wed, Aug 19, 11:48 AM
Unknown Object (File)
Wed, Aug 19, 11:48 AM
Unknown Object (File)
Wed, Aug 19, 11:48 AM
Unknown Object (File)
Wed, Aug 19, 11:48 AM
View All Files
Subscribers
imp
np

Details

Reviewers
ngie
glebius
Commits
rS298090: Add a test for cancelling an active AIO request on a socket.
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
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 1400
Build 1405: arc lint + arc unit

Event Timeline

jhb updated this revision to Diff 10713.Dec 3 2015, 11:08 PM
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.
Herald added a subscriber: imp. · View Herald TranscriptDec 3 2015, 11:08 PM
jhb added a comment.Dec 3 2015, 11:10 PM

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

ngie edited edge metadata.Dec 7 2015, 8:02 PM

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

tests/sys/aio/Makefile
7

Could you please put this on a separate line?

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

ngie added a comment.Dec 7 2015, 8:03 PM
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.

jhb added a comment.Mar 18 2016, 10:28 PM
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 updated this revision to Diff 14435.Mar 19 2016, 12:26 AM
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 revision to Diff 14481.Mar 21 2016, 6:41 PM
jhb updated this object.
  • Style and misc fixes.
jhb updated this revision to Diff 14756.Mar 30 2016, 11:53 PM
  • Rebase
  • Move test over into aio_test.
ngie added inline comments.Apr 6 2016, 4:16 AM
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...

jhb added inline comments.Apr 8 2016, 9:01 PM
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.

jhb updated this revision to Diff 15258.Apr 15 2016, 11:56 PM
  • Drop OSRELDATE macros and test __FreeBSD_version at compile time.
Closed by commit rS298090: Add a test for cancelling an active AIO request on a socket. (authored by jhb). · Explain WhyApr 16 2016, 12:01 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
sys/
kern/
54 lines
tests/
sys/
aio/
2 lines
99 lines

Diff 10713

View Options

sys/kern/vfs_aio.c

Loading...
View Options

tests/sys/aio/Makefile

Loading...
View Options

tests/sys/aio/aio_socket_test.c

Loading...