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
