[PATCH net-next v2 15/17] iscsi: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage

10 views
Skip to first unread message

David Howells

unread,
Jun 17, 2023, 8:13:04 AM6/17/23
to net...@vger.kernel.org, David Howells, Alexander Duyck, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe, linu...@kvack.org, linux-...@vger.kernel.org, Lee Duncan, Chris Leech, Mike Christie, Maurizio Lombardi, James E.J. Bottomley, Martin K. Petersen, Al Viro, open-...@googlegroups.com, linux...@vger.kernel.org, target...@vger.kernel.org
Use sendmsg() with MSG_SPLICE_PAGES rather than sendpage. This allows
multiple pages and multipage folios to be passed through.

TODO: iscsit_fe_sendpage_sg() should perhaps set up a bio_vec array for the
entire set of pages it's going to transfer plus two for the header and
trailer and page fragments to hold the header and trailer - and then call
sendmsg once for the entire message.

Signed-off-by: David Howells <dhow...@redhat.com>
cc: Lee Duncan <ldu...@suse.com>
cc: Chris Leech <cle...@redhat.com>
cc: Mike Christie <michael....@oracle.com>
cc: Maurizio Lombardi <mlom...@redhat.com>
cc: "James E.J. Bottomley" <je...@linux.ibm.com>
cc: "Martin K. Petersen" <martin....@oracle.com>
cc: "David S. Miller" <da...@davemloft.net>
cc: Eric Dumazet <edum...@google.com>
cc: Jakub Kicinski <ku...@kernel.org>
cc: Paolo Abeni <pab...@redhat.com>
cc: Jens Axboe <ax...@kernel.dk>
cc: Matthew Wilcox <wi...@infradead.org>
cc: Al Viro <vi...@zeniv.linux.org.uk>
cc: open-...@googlegroups.com
cc: linux...@vger.kernel.org
cc: target...@vger.kernel.org
cc: net...@vger.kernel.org
---

Notes:
ver #2)
- Wrap lines at 80.

drivers/scsi/iscsi_tcp.c | 26 +++++++++---------------
drivers/scsi/iscsi_tcp.h | 2 +-
drivers/target/iscsi/iscsi_target_util.c | 15 ++++++++------
3 files changed, 20 insertions(+), 23 deletions(-)

diff --git a/drivers/scsi/iscsi_tcp.c b/drivers/scsi/iscsi_tcp.c
index 9637d4bc2bc9..9ab8555180a3 100644
--- a/drivers/scsi/iscsi_tcp.c
+++ b/drivers/scsi/iscsi_tcp.c
@@ -301,35 +301,32 @@ static int iscsi_sw_tcp_xmit_segment(struct iscsi_tcp_conn *tcp_conn,

while (!iscsi_tcp_segment_done(tcp_conn, segment, 0, r)) {
struct scatterlist *sg;
+ struct msghdr msg = {};
+ struct bio_vec bv;
unsigned int offset, copy;
- int flags = 0;

r = 0;
offset = segment->copied;
copy = segment->size - offset;

if (segment->total_copied + segment->size < segment->total_size)
- flags |= MSG_MORE | MSG_SENDPAGE_NOTLAST;
+ msg.msg_flags |= MSG_MORE;

if (tcp_sw_conn->queue_recv)
- flags |= MSG_DONTWAIT;
+ msg.msg_flags |= MSG_DONTWAIT;

- /* Use sendpage if we can; else fall back to sendmsg */
if (!segment->data) {
+ if (!tcp_conn->iscsi_conn->datadgst_en)
+ msg.msg_flags |= MSG_SPLICE_PAGES;
sg = segment->sg;
offset += segment->sg_offset + sg->offset;
- r = tcp_sw_conn->sendpage(sk, sg_page(sg), offset,
- copy, flags);
+ bvec_set_page(&bv, sg_page(sg), copy, offset);
} else {
- struct msghdr msg = { .msg_flags = flags };
- struct kvec iov = {
- .iov_base = segment->data + offset,
- .iov_len = copy
- };
-
- r = kernel_sendmsg(sk, &msg, &iov, 1, copy);
+ bvec_set_virt(&bv, segment->data + offset, copy);
}
+ iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bv, 1, copy);

+ r = sock_sendmsg(sk, &msg);
if (r < 0) {
iscsi_tcp_segment_unmap(segment);
return r;
@@ -746,7 +743,6 @@ iscsi_sw_tcp_conn_bind(struct iscsi_cls_session *cls_session,
sock_no_linger(sk);

iscsi_sw_tcp_conn_set_callbacks(conn);
- tcp_sw_conn->sendpage = tcp_sw_conn->sock->ops->sendpage;
/*
* set receive state machine into initial state
*/
@@ -777,8 +773,6 @@ static int iscsi_sw_tcp_conn_set_param(struct iscsi_cls_conn *cls_conn,
return -ENOTCONN;
}
iscsi_set_param(cls_conn, param, buf, buflen);
- tcp_sw_conn->sendpage = conn->datadgst_en ?
- sock_no_sendpage : tcp_sw_conn->sock->ops->sendpage;
mutex_unlock(&tcp_sw_conn->sock_lock);
break;
case ISCSI_PARAM_MAX_R2T:
diff --git a/drivers/scsi/iscsi_tcp.h b/drivers/scsi/iscsi_tcp.h
index 68e14a344904..d6ec08d7eb63 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -48,7 +48,7 @@ struct iscsi_sw_tcp_conn {
uint32_t sendpage_failures_cnt;
uint32_t discontiguous_hdr_cnt;

- ssize_t (*sendpage)(struct socket *, struct page *, int, size_t, int);
+ bool can_splice_to_tcp;
};

struct iscsi_sw_tcp_host {
diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
index b14835fcb033..6231fa4ef5c6 100644
--- a/drivers/target/iscsi/iscsi_target_util.c
+++ b/drivers/target/iscsi/iscsi_target_util.c
@@ -1129,6 +1129,8 @@ int iscsit_fe_sendpage_sg(
struct iscsit_conn *conn)
{
struct scatterlist *sg = cmd->first_data_sg;
+ struct bio_vec bvec;
+ struct msghdr msghdr = { .msg_flags = MSG_SPLICE_PAGES, };
struct kvec iov;
u32 tx_hdr_size, data_len;
u32 offset = cmd->first_data_sg_off;
@@ -1172,17 +1174,18 @@ int iscsit_fe_sendpage_sg(
u32 space = (sg->length - offset);
u32 sub_len = min_t(u32, data_len, space);
send_pg:
- tx_sent = conn->sock->ops->sendpage(conn->sock,
- sg_page(sg), sg->offset + offset, sub_len, 0);
+ bvec_set_page(&bvec, sg_page(sg), sub_len, sg->offset + offset);
+ iov_iter_bvec(&msghdr.msg_iter, ITER_SOURCE, &bvec, 1, sub_len);
+
+ tx_sent = conn->sock->ops->sendmsg(conn->sock, &msghdr,
+ sub_len);
if (tx_sent != sub_len) {
if (tx_sent == -EAGAIN) {
- pr_err("tcp_sendpage() returned"
- " -EAGAIN\n");
+ pr_err("sendmsg/splice returned -EAGAIN\n");
goto send_pg;
}

- pr_err("tcp_sendpage() failure: %d\n",
- tx_sent);
+ pr_err("sendmsg/splice failure: %d\n", tx_sent);
return -1;
}


David Howells

unread,
Jun 20, 2023, 10:57:20 AM6/20/23
to net...@vger.kernel.org, David Howells, Alexander Duyck, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe, linu...@kvack.org, linux-...@vger.kernel.org, Lee Duncan, Chris Leech, Mike Christie, Maurizio Lombardi, James E.J. Bottomley, Martin K. Petersen, Al Viro, open-...@googlegroups.com, linux...@vger.kernel.org, target...@vger.kernel.org

David Howells

unread,
Jun 23, 2023, 7:45:19 AM6/23/23
to net...@vger.kernel.org, David Howells, Alexander Duyck, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe, linu...@kvack.org, linux-...@vger.kernel.org, Lee Duncan, Chris Leech, Mike Christie, Maurizio Lombardi, James E.J. Bottomley, Martin K. Petersen, Al Viro, open-...@googlegroups.com, linux...@vger.kernel.org, target...@vger.kernel.org
drivers/scsi/iscsi_tcp.h | 2 --
drivers/target/iscsi/iscsi_target_util.c | 15 ++++++++------
3 files changed, 19 insertions(+), 24 deletions(-)
index 68e14a344904..89a6fc552f0b 100644
--- a/drivers/scsi/iscsi_tcp.h
+++ b/drivers/scsi/iscsi_tcp.h
@@ -47,8 +47,6 @@ struct iscsi_sw_tcp_conn {
/* MIB custom statistics */
uint32_t sendpage_failures_cnt;
uint32_t discontiguous_hdr_cnt;
-
- ssize_t (*sendpage)(struct socket *, struct page *, int, size_t, int);

Mike Christie

unread,
Jun 23, 2023, 12:04:03 PM6/23/23
to David Howells, net...@vger.kernel.org, Alexander Duyck, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe, linu...@kvack.org, linux-...@vger.kernel.org, Lee Duncan, Chris Leech, Maurizio Lombardi, James E.J. Bottomley, Martin K. Petersen, Al Viro, open-...@googlegroups.com, linux...@vger.kernel.org, target...@vger.kernel.org, Dimitri KRAVTCHUK
When you send this again can you split it into 2 patches?

drivers/scsi/iscsi_tcp.* is one driver. It's similar to a NFS client and
it has a set of maintainers that you saw in the MAINTAINER file.

drivers/target/iscsi/iscsi_target_util.c is another driver which is similar
to a NFS server. Martin is the overall maintainer but it's a group effort
to review patches.

I've tested and reviewed the iscsi_tcp parts. You can add my:

Reviewed-by: Mike Christie <michael....@oracle.com>

For the iscsi_target_util.c part, I'm reviewing it now and hoping Maurizio
and/or Dimitry might review as well.

One question on the target part I had is about the TODO above. Is that
something you were going to do, or is it something you are asking the target
people to do?

David Howells

unread,
Jun 23, 2023, 3:46:10 PM6/23/23
to Mike Christie, dhow...@redhat.com, net...@vger.kernel.org, Alexander Duyck, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Willem de Bruijn, David Ahern, Matthew Wilcox, Jens Axboe, linu...@kvack.org, linux-...@vger.kernel.org, Lee Duncan, Chris Leech, Maurizio Lombardi, James E.J. Bottomley, Martin K. Petersen, Al Viro, open-...@googlegroups.com, linux...@vger.kernel.org, target...@vger.kernel.org, Dimitri KRAVTCHUK
Mike Christie <michael....@oracle.com> wrote:

>
> One question on the target part I had is about the TODO above. Is that
> something you were going to do, or is it something you are asking the target
> people to do?

I've got an in-progress patch for that, but it's not the simplest code to
modify. I'm holding off on completing it till the simpler cleanup is in. I
might end up having to push the incomplete patch your way to ask for advice on
how to complete it.

David

Reply all
Reply to author
Forward
0 new messages