Skip to content

Commit f279cd0

Browse files
almostivantrondmypd
almostivan
authored and
trondmypd
committed
rpc: return sent and err from xs_sendpages()
If an error is returned after the first bits of a packet have already been successfully queued, xs_sendpages() will return a positive 'int' value indicating success. Callers seem to treat this as -EAGAIN. However, there are cases where its not a question of waiting for the write queue to drain. For example, when there is an iptables rule dropping packets to the destination, the lower level code can return -EPERM only after parts of the packet have been successfully queued. In this case, we can end up continuously retrying resulting in a kernel softlockup. This patch is intended to make no changes in behavior but is in preparation for subsequent patches that can make decisions based on both on the number of bytes sent by xs_sendpages() and any errors that may have be returned. Signed-off-by: Jason Baron <jbaron@akamai.com> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
1 parent 173b3af commit f279cd0

File tree

1 file changed

+42
-39
lines changed

1 file changed

+42
-39
lines changed

‎net/sunrpc/xprtsock.c

Lines changed: 42 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -399,13 +399,13 @@ static int xs_send_kvec(struct socket *sock, struct sockaddr *addr, int addrlen,
399399
return kernel_sendmsg(sock, &msg, NULL, 0, 0);
400400
}
401401

402-
static int xs_send_pagedata(struct socket *sock, struct xdr_buf *xdr, unsigned int base, int more, bool zerocopy)
402+
static int xs_send_pagedata(struct socket *sock, struct xdr_buf *xdr, unsigned int base, int more, bool zerocopy, int *sent_p)
403403
{
404404
ssize_t (*do_sendpage)(struct socket *sock, struct page *page,
405405
int offset, size_t size, int flags);
406406
struct page **ppage;
407407
unsigned int remainder;
408-
int err, sent = 0;
408+
int err;
409409

410410
remainder = xdr->page_len - base;
411411
base += xdr->page_base;
@@ -424,15 +424,15 @@ static int xs_send_pagedata(struct socket *sock, struct xdr_buf *xdr, unsigned i
424424
err = do_sendpage(sock, *ppage, base, len, flags);
425425
if (remainder == 0 || err != len)
426426
break;
427-
sent += err;
427+
*sent_p += err;
428428
ppage++;
429429
base = 0;
430430
}
431-
if (sent == 0)
432-
return err;
433-
if (err > 0)
434-
sent += err;
435-
return sent;
431+
if (err > 0) {
432+
*sent_p += err;
433+
err = 0;
434+
}
435+
return err;
436436
}
437437

438438
/**
@@ -443,12 +443,14 @@ static int xs_send_pagedata(struct socket *sock, struct xdr_buf *xdr, unsigned i
443443
* @xdr: buffer containing this request
444444
* @base: starting position in the buffer
445445
* @zerocopy: true if it is safe to use sendpage()
446+
* @sent_p: return the total number of bytes successfully queued for sending
446447
*
447448
*/
448-
static int xs_sendpages(struct socket *sock, struct sockaddr *addr, int addrlen, struct xdr_buf *xdr, unsigned int base, bool zerocopy)
449+
static int xs_sendpages(struct socket *sock, struct sockaddr *addr, int addrlen, struct xdr_buf *xdr, unsigned int base, bool zerocopy, int *sent_p)
449450
{
450451
unsigned int remainder = xdr->len - base;
451-
int err, sent = 0;
452+
int err = 0;
453+
int sent = 0;
452454

453455
if (unlikely(!sock))
454456
return -ENOTSOCK;
@@ -465,31 +467,31 @@ static int xs_sendpages(struct socket *sock, struct sockaddr *addr, int addrlen,
465467
err = xs_send_kvec(sock, addr, addrlen, &xdr->head[0], base, remainder != 0);
466468
if (remainder == 0 || err != len)
467469
goto out;
468-
sent += err;
470+
*sent_p += err;
469471
base = 0;
470472
} else
471473
base -= xdr->head[0].iov_len;
472474

473475
if (base < xdr->page_len) {
474476
unsigned int len = xdr->page_len - base;
475477
remainder -= len;
476-
err = xs_send_pagedata(sock, xdr, base, remainder != 0, zerocopy);
477-
if (remainder == 0 || err != len)
478+
err = xs_send_pagedata(sock, xdr, base, remainder != 0, zerocopy, &sent);
479+
*sent_p += sent;
480+
if (remainder == 0 || sent != len)
478481
goto out;
479-
sent += err;
480482
base = 0;
481483
} else
482484
base -= xdr->page_len;
483485

484486
if (base >= xdr->tail[0].iov_len)
485-
return sent;
487+
return 0;
486488
err = xs_send_kvec(sock, NULL, 0, &xdr->tail[0], base, 0);
487489
out:
488-
if (sent == 0)
489-
return err;
490-
if (err > 0)
491-
sent += err;
492-
return sent;
490+
if (err > 0) {
491+
*sent_p += err;
492+
err = 0;
493+
}
494+
return err;
493495
}
494496

495497
static void xs_nospace_callback(struct rpc_task *task)
@@ -573,19 +575,20 @@ static int xs_local_send_request(struct rpc_task *task)
573575
container_of(xprt, struct sock_xprt, xprt);
574576
struct xdr_buf *xdr = &req->rq_snd_buf;
575577
int status;
578+
int sent = 0;
576579

577580
xs_encode_stream_record_marker(&req->rq_snd_buf);
578581

579582
xs_pktdump("packet data:",
580583
req->rq_svec->iov_base, req->rq_svec->iov_len);
581584

582-
status = xs_sendpages(transport->sock, NULL, 0,
583-
xdr, req->rq_bytes_sent, true);
585+
status = xs_sendpages(transport->sock, NULL, 0, xdr, req->rq_bytes_sent,
586+
true, &sent);
584587
dprintk("RPC: %s(%u) = %d\n",
585588
__func__, xdr->len - req->rq_bytes_sent, status);
586-
if (likely(status >= 0)) {
587-
req->rq_bytes_sent += status;
588-
req->rq_xmit_bytes_sent += status;
589+
if (likely(sent > 0) || status == 0) {
590+
req->rq_bytes_sent += sent;
591+
req->rq_xmit_bytes_sent += sent;
589592
if (likely(req->rq_bytes_sent >= req->rq_slen)) {
590593
req->rq_bytes_sent = 0;
591594
return 0;
@@ -626,6 +629,7 @@ static int xs_udp_send_request(struct rpc_task *task)
626629
struct rpc_xprt *xprt = req->rq_xprt;
627630
struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt);
628631
struct xdr_buf *xdr = &req->rq_snd_buf;
632+
int sent = 0;
629633
int status;
630634

631635
xs_pktdump("packet data:",
@@ -634,17 +638,15 @@ static int xs_udp_send_request(struct rpc_task *task)
634638

635639
if (!xprt_bound(xprt))
636640
return -ENOTCONN;
637-
status = xs_sendpages(transport->sock,
638-
xs_addr(xprt),
639-
xprt->addrlen, xdr,
640-
req->rq_bytes_sent, true);
641+
status = xs_sendpages(transport->sock, xs_addr(xprt), xprt->addrlen,
642+
xdr, req->rq_bytes_sent, true, &sent);
641643

642644
dprintk("RPC: xs_udp_send_request(%u) = %d\n",
643645
xdr->len - req->rq_bytes_sent, status);
644646

645-
if (status >= 0) {
646-
req->rq_xmit_bytes_sent += status;
647-
if (status >= req->rq_slen)
647+
if (sent > 0 || status == 0) {
648+
req->rq_xmit_bytes_sent += sent;
649+
if (sent >= req->rq_slen)
648650
return 0;
649651
/* Still some bytes left; set up for a retry later. */
650652
status = -EAGAIN;
@@ -713,6 +715,7 @@ static int xs_tcp_send_request(struct rpc_task *task)
713715
struct xdr_buf *xdr = &req->rq_snd_buf;
714716
bool zerocopy = true;
715717
int status;
718+
int sent;
716719

717720
xs_encode_stream_record_marker(&req->rq_snd_buf);
718721

@@ -730,26 +733,26 @@ static int xs_tcp_send_request(struct rpc_task *task)
730733
* to cope with writespace callbacks arriving _after_ we have
731734
* called sendmsg(). */
732735
while (1) {
733-
status = xs_sendpages(transport->sock,
734-
NULL, 0, xdr, req->rq_bytes_sent,
735-
zerocopy);
736+
sent = 0;
737+
status = xs_sendpages(transport->sock, NULL, 0, xdr,
738+
req->rq_bytes_sent, zerocopy, &sent);
736739

737740
dprintk("RPC: xs_tcp_send_request(%u) = %d\n",
738741
xdr->len - req->rq_bytes_sent, status);
739742

740-
if (unlikely(status < 0))
743+
if (unlikely(sent == 0 && status < 0))
741744
break;
742745

743746
/* If we've sent the entire packet, immediately
744747
* reset the count of bytes sent. */
745-
req->rq_bytes_sent += status;
746-
req->rq_xmit_bytes_sent += status;
748+
req->rq_bytes_sent += sent;
749+
req->rq_xmit_bytes_sent += sent;
747750
if (likely(req->rq_bytes_sent >= req->rq_slen)) {
748751
req->rq_bytes_sent = 0;
749752
return 0;
750753
}
751754

752-
if (status != 0)
755+
if (sent != 0)
753756
continue;
754757
status = -EAGAIN;
755758
break;

0 commit comments

Comments
 (0)