| Message ID | 1407085393-3175-5-git-send-email-trond.myklebust@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Sun, Aug 03, 2014 at 01:03:06PM -0400, Trond Myklebust wrote: > We already determined that there was enough wspace when we > called svc_xprt_enqueue. So xpo_has_wspace may have returned true then, but I don't see what guarantees it still would now. Couldn't another server thread have also run svc_recv() and the atomic_add(rqstp->rq_reserved, &xprt->xpt_reserved) between the svc_xprt_enqueue call and now? --b. > > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> > --- > net/sunrpc/svc_xprt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c > index 32647b2a6a34..b731077b6a30 100644 > --- a/net/sunrpc/svc_xprt.c > +++ b/net/sunrpc/svc_xprt.c > @@ -744,7 +744,7 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt) > svc_add_new_temp_xprt(serv, newxpt); > else > module_put(xprt->xpt_class->xcl_owner); > - } else if (xprt->xpt_ops->xpo_has_wspace(xprt)) { > + } else { > /* XPT_DATA|XPT_DEFERRED case: */ > dprintk("svc: server %p, pool %u, transport %p, inuse=%d\n", > rqstp, rqstp->rq_pool->sp_id, xprt, > -- > 1.9.3 > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Aug 12, 2014 at 3:16 PM, Bruce Fields <bfields@fieldses.org> wrote: > On Sun, Aug 03, 2014 at 01:03:06PM -0400, Trond Myklebust wrote: >> We already determined that there was enough wspace when we >> called svc_xprt_enqueue. > > So xpo_has_wspace may have returned true then, but I don't see what > guarantees it still would now. Couldn't another server thread have also > run svc_recv() and the atomic_add(rqstp->rq_reserved, > &xprt->xpt_reserved) between the svc_xprt_enqueue call and now? The point is that all this is just a heuristic: the TCP send window can collapse at any time. So rather than waste cycles doing multiple redundant tests that ultimately mean nothing when you call sendmsg(), do it once and be damned... > --b. > >> >> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> >> --- >> net/sunrpc/svc_xprt.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c >> index 32647b2a6a34..b731077b6a30 100644 >> --- a/net/sunrpc/svc_xprt.c >> +++ b/net/sunrpc/svc_xprt.c >> @@ -744,7 +744,7 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt) >> svc_add_new_temp_xprt(serv, newxpt); >> else >> module_put(xprt->xpt_class->xcl_owner); >> - } else if (xprt->xpt_ops->xpo_has_wspace(xprt)) { >> + } else { >> /* XPT_DATA|XPT_DEFERRED case: */ >> dprintk("svc: server %p, pool %u, transport %p, inuse=%d\n", >> rqstp, rqstp->rq_pool->sp_id, xprt, >> -- >> 1.9.3 >>
On Tue, Aug 12, 2014 at 03:31:43PM -0400, Trond Myklebust wrote: > On Tue, Aug 12, 2014 at 3:16 PM, Bruce Fields <bfields@fieldses.org> wrote: > > On Sun, Aug 03, 2014 at 01:03:06PM -0400, Trond Myklebust wrote: > >> We already determined that there was enough wspace when we > >> called svc_xprt_enqueue. > > > > So xpo_has_wspace may have returned true then, but I don't see what > > guarantees it still would now. Couldn't another server thread have also > > run svc_recv() and the atomic_add(rqstp->rq_reserved, > > &xprt->xpt_reserved) between the svc_xprt_enqueue call and now? > > The point is that all this is just a heuristic: the TCP send window > can collapse at any time. So rather than waste cycles doing multiple > redundant tests that ultimately mean nothing when you call sendmsg(), > do it once and be damned... OK, makes sense. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c index 32647b2a6a34..b731077b6a30 100644 --- a/net/sunrpc/svc_xprt.c +++ b/net/sunrpc/svc_xprt.c @@ -744,7 +744,7 @@ static int svc_handle_xprt(struct svc_rqst *rqstp, struct svc_xprt *xprt) svc_add_new_temp_xprt(serv, newxpt); else module_put(xprt->xpt_class->xcl_owner); - } else if (xprt->xpt_ops->xpo_has_wspace(xprt)) { + } else { /* XPT_DATA|XPT_DEFERRED case: */ dprintk("svc: server %p, pool %u, transport %p, inuse=%d\n", rqstp, rqstp->rq_pool->sp_id, xprt,
We already determined that there was enough wspace when we called svc_xprt_enqueue. Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com> --- net/sunrpc/svc_xprt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)