On Tue, Dec 16, 2014 at 04:48:58AM +0000, Nicholas A. Bellinger wrote: > In practice this has not been an issue because iscsit_do_tx_data() > is only used for transferring 48 byte headers + 4 byte digests, > along with seldom used control payloads from NOPIN + TEXT_RSP + > REJECT with less than 32k of data. Nor has it been occuring with > iscsit_do_rx_data() because MSG_WAITALL won't return to caller > until the requested transfer length is reached, or an error has > occured. > > So following Al's audit of iovec consumers, go ahead and fail > the connection on short writes/reads for now, and remove the > bogus logic. Umm... This won't apply anymore. For one thing, rscvmsg path in mainline doesn't use kernel_recvmsg() - not since commit e5a4b0bb803b39a36478451eae53a880d2663d5b Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx> Date: Mon Nov 24 18:17:55 2014 -0500 switch memcpy_to_msg() and skb_copy{,_and_csum}_datagram_msg() to primitives That code has already become memset(&msg, 0, sizeof(struct msghdr)); iov_iter_kvec(&msg.msg_iter, READ | ITER_KVEC, count->iov, count->iov_count, data); while (total_rx < data) { rx_loop = sock_recvmsg(conn->sock, &msg, (data - total_rx), MSG_WAITALL); if (rx_loop <= 0) { pr_debug("rx_loop: %d total_rx: %d\n", rx_loop, total_rx); return rx_loop; } total_rx += rx_loop; pr_debug("rx_loop: %d, total_rx: %d, data: %d\n", rx_loop, total_rx, data); } with short reads dealt with just fine - we set ->msg_iter once and each call of sock_recvmsg() (which doesn't need set_fs() anymore) advances it for the amount actually received. sendmsg() side is trivially dealt with in the same fashion. I haven't pushed that into vfs#iov_iter-net yet, but as soon as the davem opens net-next I'll be posting the sendmsg part of the series for review and this will go there as well. FWIW, right now it looks thus: iov_iter_kvec(&msg.msg_iter, WRITE | ITER_KVEC, count->iov, count->iov_count, data); while (msg_data_left(&msg)) { tx_loop = sock_sendmsg(conn->sock, &msg); if (tx_loop <= 0) { pr_debug("tx_loop: %d total_tx %d\n", tx_loop, total_tx); return tx_loop; } total_tx += tx_loop; pr_debug("tx_loop: %d, total_tx: %d, data: %d\n", tx_loop, total_tx, data); } (msg_data_left(msg) == iov_iter_count(&msg->msg_iter) and sock_sendmsg() has lost the third argument - it was always equal to msg_data_left(msg)). iovec is never drained, ->msg_iter is always advanced by the amount actually sent. Makes (ex-)users of kernel_sendmsg()/kernel_recvmsg() much simpler and trivial way of handling short writes/reads becomes correct... -- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html