Date: Tue, 24 May 2016 21:09:05 +0000 (UTC) From: John Baldwin <jhb@FreeBSD.org> To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r300626 - in head: sys/kern tests/sys/aio Message-ID: <201605242109.u4OL95Fr033351@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help
Author: jhb Date: Tue May 24 21:09:05 2016 New Revision: 300626 URL: https://svnweb.freebsd.org/changeset/base/300626 Log: Return the correct status when a partially completed request is cancelled. After the previous changes to fix requests on blocking sockets to complete across multiple operations, an edge case exists where a request can be cancelled after it has partially completed. POSIX doesn't appear to dictate exactly how to handle this case, but in general I feel that aio_cancel() should arrange to cancel any request it can, but that any partially completed requests should return a partial completion rather than ECANCELED. To that end, fix the socket AIO cancellation routine to return a short read/write if a partially completed request is cancelled rather than ECANCELED. Sponsored by: Chelsio Communications Modified: head/sys/kern/sys_socket.c head/tests/sys/aio/aio_test.c Modified: head/sys/kern/sys_socket.c ============================================================================== --- head/sys/kern/sys_socket.c Tue May 24 20:06:41 2016 (r300625) +++ head/sys/kern/sys_socket.c Tue May 24 21:09:05 2016 (r300626) @@ -721,6 +721,7 @@ soo_aio_cancel(struct kaiocb *job) { struct socket *so; struct sockbuf *sb; + long done; int opcode; so = job->fd_file->f_data; @@ -739,7 +740,11 @@ soo_aio_cancel(struct kaiocb *job) sb->sb_flags &= ~SB_AIO; SOCKBUF_UNLOCK(sb); - aio_cancel(job); + done = job->uaiocb._aiocb_private.status; + if (done != 0) + aio_complete(job, done, 0); + else + aio_cancel(job); } static int Modified: head/tests/sys/aio/aio_test.c ============================================================================== --- head/tests/sys/aio/aio_test.c Tue May 24 20:06:41 2016 (r300625) +++ head/tests/sys/aio/aio_test.c Tue May 24 21:09:05 2016 (r300626) @@ -845,6 +845,74 @@ ATF_TC_BODY(aio_socket_blocking_short_wr close(s[0]); } +/* + * This test verifies that cancelling a partially completed socket write + * returns a short write rather than ECANCELED. + */ +ATF_TC_WITHOUT_HEAD(aio_socket_short_write_cancel); +ATF_TC_BODY(aio_socket_short_write_cancel, tc) +{ + struct aiocb iocb, *iocbp; + char *buffer[2]; + ssize_t done; + int buffer_size, sb_size; + socklen_t len; + int s[2]; + + ATF_REQUIRE_KERNEL_MODULE("aio"); + + ATF_REQUIRE(socketpair(PF_UNIX, SOCK_STREAM, 0, s) != -1); + + len = sizeof(sb_size); + ATF_REQUIRE(getsockopt(s[0], SOL_SOCKET, SO_RCVBUF, &sb_size, &len) != + -1); + ATF_REQUIRE(len == sizeof(sb_size)); + buffer_size = sb_size; + + ATF_REQUIRE(getsockopt(s[1], SOL_SOCKET, SO_SNDBUF, &sb_size, &len) != + -1); + ATF_REQUIRE(len == sizeof(sb_size)); + if (sb_size > buffer_size) + buffer_size = sb_size; + + /* + * Use three times the size of the MAX(receive buffer, send + * buffer) for the write to ensure that the write is split up + * into multiple writes internally. The recv() ensures that + * the write has partially completed, but a remaining size of + * two buffers should ensure that the write has not completed + * fully when it is cancelled. + */ + buffer[0] = malloc(buffer_size); + ATF_REQUIRE(buffer[0] != NULL); + buffer[1] = malloc(buffer_size * 3); + ATF_REQUIRE(buffer[1] != NULL); + + srandomdev(); + aio_fill_buffer(buffer[1], buffer_size * 3, random()); + + memset(&iocb, 0, sizeof(iocb)); + iocb.aio_fildes = s[1]; + iocb.aio_buf = buffer[1]; + iocb.aio_nbytes = buffer_size * 3; + ATF_REQUIRE(aio_write(&iocb) == 0); + + done = recv(s[0], buffer[0], buffer_size, MSG_WAITALL); + ATF_REQUIRE(done == buffer_size); + + ATF_REQUIRE(aio_error(&iocb) == EINPROGRESS); + ATF_REQUIRE(aio_cancel(s[1], &iocb) == AIO_NOTCANCELED); + + done = aio_waitcomplete(&iocbp, NULL); + ATF_REQUIRE(iocbp == &iocb); + ATF_REQUIRE(done >= buffer_size && done <= buffer_size * 2); + + ATF_REQUIRE(memcmp(buffer[0], buffer[1], buffer_size) == 0); + + close(s[1]); + close(s[0]); +} + ATF_TP_ADD_TCS(tp) { @@ -857,6 +925,7 @@ ATF_TP_ADD_TCS(tp) ATF_TP_ADD_TC(tp, aio_large_read_test); ATF_TP_ADD_TC(tp, aio_socket_two_reads); ATF_TP_ADD_TC(tp, aio_socket_blocking_short_write); + ATF_TP_ADD_TC(tp, aio_socket_short_write_cancel); return (atf_no_error()); }
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201605242109.u4OL95Fr033351>