On Wed, Jul 23, 2025 at 02:52:12PM +0000, Bernard Metzler wrote:
-----Original Message----- From: Pedro Falcato pfalcato@suse.de Sent: Wednesday, 23 July 2025 12:41 To: Jason Gunthorpe jgg@ziepe.ca; Bernard Metzler BMT@zurich.ibm.com; Leon Romanovsky leon@kernel.org; Vlastimil Babka vbabka@suse.cz Cc: Jakub Kicinski kuba@kernel.org; David Howells dhowells@redhat.com; Tom Talpey tom@talpey.com; linux-rdma@vger.kernel.org; linux- kernel@vger.kernel.org; linux-mm@kvack.org; Pedro Falcato pfalcato@suse.de; stable@vger.kernel.org; kernel test robot oliver.sang@intel.com
[snip]
drivers/infiniband/sw/siw/siw_qp_tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/sw/siw/siw_qp_tx.c b/drivers/infiniband/sw/siw/siw_qp_tx.c index 3a08f57d2211..9576a2b766c4 100644 --- a/drivers/infiniband/sw/siw/siw_qp_tx.c +++ b/drivers/infiniband/sw/siw/siw_qp_tx.c @@ -340,11 +340,11 @@ static int siw_tcp_sendpages(struct socket *s, struct page **page, int offset, if (!sendpage_ok(page[i])) msg.msg_flags &= ~MSG_SPLICE_PAGES; bvec_set_page(&bvec, page[i], bytes, offset);
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, size);
iov_iter_bvec(&msg.msg_iter, ITER_SOURCE, &bvec, 1, bytes);try_page_again: lock_sock(sk);
rv = tcp_sendmsg_locked(sk, &msg, size);
rv = tcp_sendmsg_locked(sk, &msg, bytes);Pedro, many thanks for catching this! I completely missed it during my too sloppy review of that patch. It's a serious bug which must be fixed asap. BUT, looking closer, I do not see the offset being taken into account when retrying a current segment. So, resend attempts seem to send old data which are already out. Shouldn't the try_page_again: label be above bvec_set_page()??
This was raised off-list by Vlastimil - I think it's harmless to bump (but not use) the offset here, because by reusing the iov_iter we progressively consume the data (it keeps its own size and offset tracking internally). So the only thing we need to track is the size we pass to tcp_sendmsg_locked[1].
If desired (and if my logic is correct!) I can send a v2 deleting that bit.
[1] Assuming tcp_sendmsg_locked guarantees it will never consume something out of the iovec_iter without reporting it as bytes copied, which from a code reading it seems like it won't...