Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley
        Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United
        Kingdom.
        Registered in England and Wales under Company Registration No. 3798903
From:   David Howells <dhowells@redhat.com>
In-Reply-To: <SA0PR15MB3919AD9D232B3CA789A3FD6F99939@SA0PR15MB3919.namprd15.prod.outlook.com>
References: <SA0PR15MB3919AD9D232B3CA789A3FD6F99939@SA0PR15MB3919.namprd15.prod.outlook.com> <20230331160914.1608208-1-dhowells@redhat.com> <20230331160914.1608208-39-dhowells@redhat.com>
To:     Bernard Metzler <BMT@zurich.ibm.com>
Cc:     dhowells@redhat.com, "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Al Viro <viro@zeniv.linux.org.uk>,
        Christoph Hellwig <hch@infradead.org>,
        Jens Axboe <axboe@kernel.dk>, Jeff Layton <jlayton@kernel.org>,
        Christian Brauner <brauner@kernel.org>,
        Chuck Lever III <chuck.lever@oracle.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        Tom Talpey <tom@talpey.com>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>
Subject: Re: [PATCH v3 38/55] siw: Use sendmsg(MSG_SPLICE_PAGES) rather than sendpage to transmit
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <2904968.1680682716.1@warthog.procyon.org.uk>
Content-Transfer-Encoding: quoted-printable
Date:   Wed, 05 Apr 2023 09:18:36 +0100
Message-ID: <2904969.1680682716@warthog.procyon.org.uk>
Precedence: bulk

Bernard Metzler <BMT@zurich.ibm.com> wrote:

> >  	if (c_tx->state =3D=3D SIW_SEND_HDR) {
> >  		if (c_tx->use_sendpage) {
> > @@ -457,10 +350,15 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx,
> > struct socket *s)
> > =

> =

> Couldn't we now collapse the two header handling paths
> into one, avoiding extra =

> 'if (c_tx->use_sendpage) {} else {}' conditions?

Okay, see the attached incremental change.

Note that the calls to page_frag_memdup() I previously added are probably =
not
going to be necessary as copying unspliceable data is now done in the
protocols (TCP, IP/UDP, UNIX, etc.).  See patch 08 for the TCP version.

David
---
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw=
/siw/siw_qp_tx.c
index 28076832da20..edf66a97cf5f 100644
--- a/drivers/infiniband/sw/siw/siw_qp_tx.c
+++ b/drivers/infiniband/sw/siw/siw_qp_tx.c
@@ -335,7 +335,7 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struc=
t socket *s)
 	struct siw_sge *sge =3D &wqe->sqe.sge[c_tx->sge_idx];
 	struct bio_vec bvec[MAX_ARRAY];
 	struct msghdr msg =3D { .msg_flags =3D MSG_DONTWAIT | MSG_EOR };
-	void *trl, *t;
+	void *trl;
 =

 	int seg =3D 0, do_crc =3D c_tx->do_crc, is_kva =3D 0, rv;
 	unsigned int data_len =3D c_tx->bytes_unsent, hdr_len =3D 0, trl_len =3D=
 0,
@@ -343,25 +343,11 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, str=
uct socket *s)
 		     pbl_idx =3D c_tx->pbl_idx;
 =

 	if (c_tx->state =3D=3D SIW_SEND_HDR) {
-		if (c_tx->use_sendpage) {
-			rv =3D siw_tx_ctrl(c_tx, s, MSG_DONTWAIT | MSG_MORE);
-			if (rv)
-				goto done;
+		void *hdr =3D &c_tx->pkt.ctrl + c_tx->ctrl_sent;
 =

-			c_tx->state =3D SIW_SEND_DATA;
-		} else {
-			const void *hdr =3D &c_tx->pkt.ctrl + c_tx->ctrl_sent;
-			void *h;
-
-			rv =3D -ENOMEM;
-			hdr_len =3D c_tx->ctrl_len - c_tx->ctrl_sent;
-			h =3D page_frag_memdup(NULL, hdr, hdr_len, GFP_NOFS,
-					     ULONG_MAX);
-			if (!h)
-				goto done;
-			bvec_set_virt(&bvec[0], h, hdr_len);
-			seg =3D 1;
-		}
+		hdr_len =3D c_tx->ctrl_len - c_tx->ctrl_sent;
+		bvec_set_virt(&bvec[0], hdr, hdr_len);
+		seg =3D 1;
 	}
 =

 	wqe->processed +=3D data_len;
@@ -466,12 +452,7 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, stru=
ct socket *s)
 		trl =3D &c_tx->trailer.pad[c_tx->ctrl_sent];
 		trl_len =3D MAX_TRAILER - c_tx->ctrl_sent;
 	}
-
-	rv =3D -ENOMEM;
-	t =3D page_frag_memdup(NULL, trl, trl_len, GFP_NOFS, ULONG_MAX);
-	if (!t)
-		goto done_crc;
-	bvec_set_virt(&bvec[seg], t, trl_len);
+	bvec_set_virt(&bvec[seg], trl, trl_len);
 =

 	data_len =3D c_tx->bytes_unsent;
 =

@@ -480,7 +461,6 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, struc=
t socket *s)
 	iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, bvec, seg + 1,
 		      hdr_len + data_len + trl_len);
 	rv =3D sock_sendmsg(s, &msg);
-
 	if (rv < (int)hdr_len) {
 		/* Not even complete hdr pushed or negative rv */
 		wqe->processed -=3D data_len;
@@ -541,10 +521,6 @@ static int siw_tx_hdt(struct siw_iwarp_tx *c_tx, stru=
ct socket *s)
 	}
 done_crc:
 	c_tx->do_crc =3D 0;
-	if (c_tx->state =3D=3D SIW_SEND_HDR)
-		folio_put(page_folio(bvec[0].bv_page));
-	folio_put(page_folio(bvec[seg].bv_page));
-done:
 	return rv;
 }
 =