* [PATCHv1] sunrpc: fix write space race causing stalls @ 2016-09-16 12:28 David Vrabel 2016-09-16 16:01 ` Trond Myklebust 0 siblings, 1 reply; 9+ messages in thread From: David Vrabel @ 2016-09-16 12:28 UTC (permalink / raw) To: linux-nfs; +Cc: David Vrabel, Trond Myklebust, Anna Schumaker, netdev Write space becoming available may race with putting the task to sleep in xprt_wait_for_buffer_space(). The existing mechanism to avoid the race does not work. This (edited) partial trace illustrates the problem: [1] rpc_task_run_action: task:43546@5 ... action=call_transmit [2] xs_write_space <-xs_tcp_write_space [3] xprt_write_space <-xs_write_space [4] rpc_task_sleep: task:43546@5 ... [5] xs_write_space <-xs_tcp_write_space [1] Task 43546 runs but is out of write space. [2] Space becomes available, xs_write_space() clears the SOCKWQ_ASYNC_NOSPACE bit. [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but this has not yet been queued and the wake up is lost. [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() which queues task 43546. [5] The call to sk->sk_write_space() at the end of xs_nospace() (which is supposed to handle the above race) does not call xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and thus the task is not woken. Fix the race by have xprt_wait_for_buffer_space() check for write space after putting the task to sleep. Signed-off-by: David Vrabel <david.vrabel@citrix.com> --- include/linux/sunrpc/xprt.h | 1 + net/sunrpc/xprt.c | 4 ++++ net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- 3 files changed, 24 insertions(+), 2 deletions(-) diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h index a16070d..621e74b 100644 --- a/include/linux/sunrpc/xprt.h +++ b/include/linux/sunrpc/xprt.h @@ -129,6 +129,7 @@ struct rpc_xprt_ops { void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); void * (*buf_alloc)(struct rpc_task *task, size_t size); void (*buf_free)(void *buffer); + bool (*have_write_space)(struct rpc_xprt *task); int (*send_request)(struct rpc_task *task); void (*set_retrans_timeout)(struct rpc_task *task); void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c index ea244b2..d3c1b1e 100644 --- a/net/sunrpc/xprt.c +++ b/net/sunrpc/xprt.c @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; rpc_sleep_on(&xprt->pending, task, action); + + /* Write space notification may race with putting task to sleep. */ + if (xprt->ops->have_write_space(xprt)) + rpc_wake_up_queued_task(&xprt->pending, task); } EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c index bf16883..211de5b 100644 --- a/net/sunrpc/xprtsock.c +++ b/net/sunrpc/xprtsock.c @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) spin_unlock_bh(&xprt->transport_lock); - /* Race breaker in case memory is freed before above code is called */ - sk->sk_write_space(sk); return ret; } @@ -1679,6 +1677,22 @@ static void xs_tcp_write_space(struct sock *sk) read_unlock_bh(&sk->sk_callback_lock); } +static bool xs_udp_have_write_space(struct rpc_xprt *xprt) +{ + struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt); + struct sock *sk = transport->inet; + + return sock_writeable(sk); +} + +static bool xs_tcp_have_write_space(struct rpc_xprt *xprt) +{ + struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt); + struct sock *sk = transport->inet; + + return sk_stream_is_writeable(sk); +} + static void xs_udp_do_set_buffer_size(struct rpc_xprt *xprt) { struct sock_xprt *transport = container_of(xprt, struct sock_xprt, xprt); @@ -2664,6 +2678,7 @@ static struct rpc_xprt_ops xs_local_ops = { .connect = xs_local_connect, .buf_alloc = rpc_malloc, .buf_free = rpc_free, + .have_write_space = xs_udp_have_write_space, .send_request = xs_local_send_request, .set_retrans_timeout = xprt_set_retrans_timeout_def, .close = xs_close, @@ -2683,6 +2698,7 @@ static struct rpc_xprt_ops xs_udp_ops = { .connect = xs_connect, .buf_alloc = rpc_malloc, .buf_free = rpc_free, + .have_write_space = xs_udp_have_write_space, .send_request = xs_udp_send_request, .set_retrans_timeout = xprt_set_retrans_timeout_rtt, .timer = xs_udp_timer, @@ -2704,6 +2720,7 @@ static struct rpc_xprt_ops xs_tcp_ops = { .connect = xs_connect, .buf_alloc = rpc_malloc, .buf_free = rpc_free, + .have_write_space = xs_tcp_have_write_space, .send_request = xs_tcp_send_request, .set_retrans_timeout = xprt_set_retrans_timeout_def, .close = xs_tcp_shutdown, -- 2.1.4 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls 2016-09-16 12:28 [PATCHv1] sunrpc: fix write space race causing stalls David Vrabel @ 2016-09-16 16:01 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 16:01 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Trond Myklebust, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote: > > Write space becoming available may race with putting the task to sleep > in xprt_wait_for_buffer_space(). The existing mechanism to avoid the > race does not work. > > This (edited) partial trace illustrates the problem: > > [1] rpc_task_run_action: task:43546@5 ... action=call_transmit > [2] xs_write_space <-xs_tcp_write_space > [3] xprt_write_space <-xs_write_space > [4] rpc_task_sleep: task:43546@5 ... > [5] xs_write_space <-xs_tcp_write_space > > [1] Task 43546 runs but is out of write space. > > [2] Space becomes available, xs_write_space() clears the > SOCKWQ_ASYNC_NOSPACE bit. > > [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but > this has not yet been queued and the wake up is lost. > > [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() > which queues task 43546. > > [5] The call to sk->sk_write_space() at the end of xs_nospace() (which > is supposed to handle the above race) does not call > xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and > thus the task is not woken. > > Fix the race by have xprt_wait_for_buffer_space() check for write > space after putting the task to sleep. > > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > include/linux/sunrpc/xprt.h | 1 + > net/sunrpc/xprt.c | 4 ++++ > net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- > 3 files changed, 24 insertions(+), 2 deletions(-) > > diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h > index a16070d..621e74b 100644 > --- a/include/linux/sunrpc/xprt.h > +++ b/include/linux/sunrpc/xprt.h > @@ -129,6 +129,7 @@ struct rpc_xprt_ops { > void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); > void * (*buf_alloc)(struct rpc_task *task, size_t size); > void (*buf_free)(void *buffer); > + bool (*have_write_space)(struct rpc_xprt *task); > int (*send_request)(struct rpc_task *task); > void (*set_retrans_timeout)(struct rpc_task *task); > void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > index ea244b2..d3c1b1e 100644 > --- a/net/sunrpc/xprt.c > +++ b/net/sunrpc/xprt.c > @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) > > task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; > rpc_sleep_on(&xprt->pending, task, action); > + > + /* Write space notification may race with putting task to sleep. */ > + if (xprt->ops->have_write_space(xprt)) > + rpc_wake_up_queued_task(&xprt->pending, task); > } > EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > index bf16883..211de5b 100644 > --- a/net/sunrpc/xprtsock.c > +++ b/net/sunrpc/xprtsock.c > @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) > > spin_unlock_bh(&xprt->transport_lock); > > - /* Race breaker in case memory is freed before above code is called */ > - sk->sk_write_space(sk); > return ret; > } Instead of these callbacks, why not just add a call to sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in xs_nospace()? Won’t that fix the existing race breaker? Cheers Trond ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls @ 2016-09-16 16:01 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 16:01 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Trond Myklebust, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote: >=20 > Write space becoming available may race with putting the task to sleep > in xprt_wait_for_buffer_space(). The existing mechanism to avoid the > race does not work. >=20 > This (edited) partial trace illustrates the problem: >=20 > [1] rpc_task_run_action: task:43546@5 ... action=3Dcall_transmit > [2] xs_write_space <-xs_tcp_write_space > [3] xprt_write_space <-xs_write_space > [4] rpc_task_sleep: task:43546@5 ... > [5] xs_write_space <-xs_tcp_write_space >=20 > [1] Task 43546 runs but is out of write space. >=20 > [2] Space becomes available, xs_write_space() clears the > SOCKWQ_ASYNC_NOSPACE bit. >=20 > [3] xprt_write_space() attemts to wake xprt->snd_task (=3D=3D 43546), but > this has not yet been queued and the wake up is lost. >=20 > [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() > which queues task 43546. >=20 > [5] The call to sk->sk_write_space() at the end of xs_nospace() (which > is supposed to handle the above race) does not call > xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and > thus the task is not woken. >=20 > Fix the race by have xprt_wait_for_buffer_space() check for write > space after putting the task to sleep. >=20 > Signed-off-by: David Vrabel <david.vrabel@citrix.com> > --- > include/linux/sunrpc/xprt.h | 1 + > net/sunrpc/xprt.c | 4 ++++ > net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- > 3 files changed, 24 insertions(+), 2 deletions(-) >=20 > diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h > index a16070d..621e74b 100644 > --- a/include/linux/sunrpc/xprt.h > +++ b/include/linux/sunrpc/xprt.h > @@ -129,6 +129,7 @@ struct rpc_xprt_ops { > =09void=09=09(*connect)(struct rpc_xprt *xprt, struct rpc_task *task); > =09void *=09=09(*buf_alloc)(struct rpc_task *task, size_t size); > =09void=09=09(*buf_free)(void *buffer); > +=09bool (*have_write_space)(struct rpc_xprt *task); > =09int=09=09(*send_request)(struct rpc_task *task); > =09void=09=09(*set_retrans_timeout)(struct rpc_task *task); > =09void=09=09(*timer)(struct rpc_xprt *xprt, struct rpc_task *task); > diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c > index ea244b2..d3c1b1e 100644 > --- a/net/sunrpc/xprt.c > +++ b/net/sunrpc/xprt.c > @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *tas= k, rpc_action action) >=20 > =09task->tk_timeout =3D RPC_IS_SOFT(task) ? req->rq_timeout : 0; > =09rpc_sleep_on(&xprt->pending, task, action); > + > +=09/* Write space notification may race with putting task to sleep. */ > +=09if (xprt->ops->have_write_space(xprt)) > +=09=09rpc_wake_up_queued_task(&xprt->pending, task); > } > EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >=20 > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c > index bf16883..211de5b 100644 > --- a/net/sunrpc/xprtsock.c > +++ b/net/sunrpc/xprtsock.c > @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >=20 > =09spin_unlock_bh(&xprt->transport_lock); >=20 > -=09/* Race breaker in case memory is freed before above code is called *= / > -=09sk->sk_write_space(sk); > =09return ret; > } Instead of these callbacks, why not just add a call to sk_set_bit(SOCKWQ_AS= YNC_WAITDATA, sk) after queueing the task in xs_nospace()? Won=92t that fix= the existing race breaker? Cheers Trond ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls 2016-09-16 16:01 ` Trond Myklebust (?) @ 2016-09-16 16:41 ` David Vrabel [not found] ` <57DC20C8.5000306-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> -1 siblings, 1 reply; 9+ messages in thread From: David Vrabel @ 2016-09-16 16:41 UTC (permalink / raw) To: Trond Myklebust Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing On 16/09/16 17:01, Trond Myklebust wrote: > >> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote: >> >> Write space becoming available may race with putting the task to sleep >> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >> race does not work. >> >> This (edited) partial trace illustrates the problem: >> >> [1] rpc_task_run_action: task:43546@5 ... action=call_transmit >> [2] xs_write_space <-xs_tcp_write_space >> [3] xprt_write_space <-xs_write_space >> [4] rpc_task_sleep: task:43546@5 ... >> [5] xs_write_space <-xs_tcp_write_space >> >> [1] Task 43546 runs but is out of write space. >> >> [2] Space becomes available, xs_write_space() clears the >> SOCKWQ_ASYNC_NOSPACE bit. >> >> [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but >> this has not yet been queued and the wake up is lost. >> >> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >> which queues task 43546. >> >> [5] The call to sk->sk_write_space() at the end of xs_nospace() (which >> is supposed to handle the above race) does not call >> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >> thus the task is not woken. >> >> Fix the race by have xprt_wait_for_buffer_space() check for write >> space after putting the task to sleep. >> >> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >> --- >> include/linux/sunrpc/xprt.h | 1 + >> net/sunrpc/xprt.c | 4 ++++ >> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >> 3 files changed, 24 insertions(+), 2 deletions(-) >> >> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h >> index a16070d..621e74b 100644 >> --- a/include/linux/sunrpc/xprt.h >> +++ b/include/linux/sunrpc/xprt.h >> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >> void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); >> void * (*buf_alloc)(struct rpc_task *task, size_t size); >> void (*buf_free)(void *buffer); >> + bool (*have_write_space)(struct rpc_xprt *task); >> int (*send_request)(struct rpc_task *task); >> void (*set_retrans_timeout)(struct rpc_task *task); >> void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >> index ea244b2..d3c1b1e 100644 >> --- a/net/sunrpc/xprt.c >> +++ b/net/sunrpc/xprt.c >> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) >> >> task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; >> rpc_sleep_on(&xprt->pending, task, action); >> + >> + /* Write space notification may race with putting task to sleep. */ >> + if (xprt->ops->have_write_space(xprt)) >> + rpc_wake_up_queued_task(&xprt->pending, task); >> } >> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >> >> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >> index bf16883..211de5b 100644 >> --- a/net/sunrpc/xprtsock.c >> +++ b/net/sunrpc/xprtsock.c >> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >> >> spin_unlock_bh(&xprt->transport_lock); >> >> - /* Race breaker in case memory is freed before above code is called */ >> - sk->sk_write_space(sk); >> return ret; >> } > > Instead of these callbacks, why not just add a call to > sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in > xs_nospace()? Won’t that fix the existing race breaker? I don't see how that would help. If sk->sk_write_space was already called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to sk->sk_write_space will still be a nop. Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to set this bit when we don't know if there's space or not. David ^ permalink raw reply [flat|nested] 9+ messages in thread
[parent not found: <57DC20C8.5000306-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org>]
* Re: [PATCHv1] sunrpc: fix write space race causing stalls 2016-09-16 16:41 ` David Vrabel @ 2016-09-16 17:06 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 17:06 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 12:41, David Vrabel <david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> wrote: > > On 16/09/16 17:01, Trond Myklebust wrote: >> >>> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> wrote: >>> >>> Write space becoming available may race with putting the task to sleep >>> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >>> race does not work. >>> >>> This (edited) partial trace illustrates the problem: >>> >>> [1] rpc_task_run_action: task:43546@5 ... action=call_transmit >>> [2] xs_write_space <-xs_tcp_write_space >>> [3] xprt_write_space <-xs_write_space >>> [4] rpc_task_sleep: task:43546@5 ... >>> [5] xs_write_space <-xs_tcp_write_space >>> >>> [1] Task 43546 runs but is out of write space. >>> >>> [2] Space becomes available, xs_write_space() clears the >>> SOCKWQ_ASYNC_NOSPACE bit. >>> >>> [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but >>> this has not yet been queued and the wake up is lost. >>> >>> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >>> which queues task 43546. >>> >>> [5] The call to sk->sk_write_space() at the end of xs_nospace() (which >>> is supposed to handle the above race) does not call >>> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >>> thus the task is not woken. >>> >>> Fix the race by have xprt_wait_for_buffer_space() check for write >>> space after putting the task to sleep. >>> >>> Signed-off-by: David Vrabel <david.vrabel-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> >>> --- >>> include/linux/sunrpc/xprt.h | 1 + >>> net/sunrpc/xprt.c | 4 ++++ >>> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >>> 3 files changed, 24 insertions(+), 2 deletions(-) >>> >>> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h >>> index a16070d..621e74b 100644 >>> --- a/include/linux/sunrpc/xprt.h >>> +++ b/include/linux/sunrpc/xprt.h >>> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >>> void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); >>> void * (*buf_alloc)(struct rpc_task *task, size_t size); >>> void (*buf_free)(void *buffer); >>> + bool (*have_write_space)(struct rpc_xprt *task); >>> int (*send_request)(struct rpc_task *task); >>> void (*set_retrans_timeout)(struct rpc_task *task); >>> void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >>> index ea244b2..d3c1b1e 100644 >>> --- a/net/sunrpc/xprt.c >>> +++ b/net/sunrpc/xprt.c >>> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) >>> >>> task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; >>> rpc_sleep_on(&xprt->pending, task, action); >>> + >>> + /* Write space notification may race with putting task to sleep. */ >>> + if (xprt->ops->have_write_space(xprt)) >>> + rpc_wake_up_queued_task(&xprt->pending, task); >>> } >>> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >>> >>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >>> index bf16883..211de5b 100644 >>> --- a/net/sunrpc/xprtsock.c >>> +++ b/net/sunrpc/xprtsock.c >>> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >>> >>> spin_unlock_bh(&xprt->transport_lock); >>> >>> - /* Race breaker in case memory is freed before above code is called */ >>> - sk->sk_write_space(sk); >>> return ret; >>> } >> >> Instead of these callbacks, why not just add a call to >> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in >> xs_nospace()? Won’t that fix the existing race breaker? > > I don't see how that would help. If sk->sk_write_space was already > called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to > sk->sk_write_space will still be a nop. Sorry. Copy+paste error. I meant SOCKWQ_ASYNC_NOSPACE. > > Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to set > this bit when we don't know if there's space or not. Why not? -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls @ 2016-09-16 17:06 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 17:06 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 12:41, David Vrabel <david.vrabel@citrix.com> wrote: >=20 > On 16/09/16 17:01, Trond Myklebust wrote: >>=20 >>> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote= : >>>=20 >>> Write space becoming available may race with putting the task to sleep >>> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >>> race does not work. >>>=20 >>> This (edited) partial trace illustrates the problem: >>>=20 >>> [1] rpc_task_run_action: task:43546@5 ... action=3Dcall_transmit >>> [2] xs_write_space <-xs_tcp_write_space >>> [3] xprt_write_space <-xs_write_space >>> [4] rpc_task_sleep: task:43546@5 ... >>> [5] xs_write_space <-xs_tcp_write_space >>>=20 >>> [1] Task 43546 runs but is out of write space. >>>=20 >>> [2] Space becomes available, xs_write_space() clears the >>> SOCKWQ_ASYNC_NOSPACE bit. >>>=20 >>> [3] xprt_write_space() attemts to wake xprt->snd_task (=3D=3D 43546), b= ut >>> this has not yet been queued and the wake up is lost. >>>=20 >>> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >>> which queues task 43546. >>>=20 >>> [5] The call to sk->sk_write_space() at the end of xs_nospace() (which >>> is supposed to handle the above race) does not call >>> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >>> thus the task is not woken. >>>=20 >>> Fix the race by have xprt_wait_for_buffer_space() check for write >>> space after putting the task to sleep. >>>=20 >>> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >>> --- >>> include/linux/sunrpc/xprt.h | 1 + >>> net/sunrpc/xprt.c | 4 ++++ >>> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >>> 3 files changed, 24 insertions(+), 2 deletions(-) >>>=20 >>> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h >>> index a16070d..621e74b 100644 >>> --- a/include/linux/sunrpc/xprt.h >>> +++ b/include/linux/sunrpc/xprt.h >>> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >>> =09void=09=09(*connect)(struct rpc_xprt *xprt, struct rpc_task *task); >>> =09void *=09=09(*buf_alloc)(struct rpc_task *task, size_t size); >>> =09void=09=09(*buf_free)(void *buffer); >>> +=09bool (*have_write_space)(struct rpc_xprt *task); >>> =09int=09=09(*send_request)(struct rpc_task *task); >>> =09void=09=09(*set_retrans_timeout)(struct rpc_task *task); >>> =09void=09=09(*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >>> index ea244b2..d3c1b1e 100644 >>> --- a/net/sunrpc/xprt.c >>> +++ b/net/sunrpc/xprt.c >>> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *t= ask, rpc_action action) >>>=20 >>> =09task->tk_timeout =3D RPC_IS_SOFT(task) ? req->rq_timeout : 0; >>> =09rpc_sleep_on(&xprt->pending, task, action); >>> + >>> +=09/* Write space notification may race with putting task to sleep. */ >>> +=09if (xprt->ops->have_write_space(xprt)) >>> +=09=09rpc_wake_up_queued_task(&xprt->pending, task); >>> } >>> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >>>=20 >>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >>> index bf16883..211de5b 100644 >>> --- a/net/sunrpc/xprtsock.c >>> +++ b/net/sunrpc/xprtsock.c >>> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >>>=20 >>> =09spin_unlock_bh(&xprt->transport_lock); >>>=20 >>> -=09/* Race breaker in case memory is freed before above code is called= */ >>> -=09sk->sk_write_space(sk); >>> =09return ret; >>> } >>=20 >> Instead of these callbacks, why not just add a call to >> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in >> xs_nospace()? Won=92t that fix the existing race breaker? >=20 > I don't see how that would help. If sk->sk_write_space was already > called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to > sk->sk_write_space will still be a nop. Sorry. Copy+paste error. I meant SOCKWQ_ASYNC_NOSPACE. >=20 > Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to set > this bit when we don't know if there's space or not. Why not? ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls 2016-09-16 17:06 ` Trond Myklebust (?) @ 2016-09-16 17:29 ` David Vrabel 2016-09-16 18:04 ` Trond Myklebust -1 siblings, 1 reply; 9+ messages in thread From: David Vrabel @ 2016-09-16 17:29 UTC (permalink / raw) To: Trond Myklebust Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing On 16/09/16 18:06, Trond Myklebust wrote: > >> On Sep 16, 2016, at 12:41, David Vrabel <david.vrabel@citrix.com> wrote: >> >> On 16/09/16 17:01, Trond Myklebust wrote: >>> >>>> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote: >>>> >>>> Write space becoming available may race with putting the task to sleep >>>> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >>>> race does not work. >>>> >>>> This (edited) partial trace illustrates the problem: >>>> >>>> [1] rpc_task_run_action: task:43546@5 ... action=call_transmit >>>> [2] xs_write_space <-xs_tcp_write_space >>>> [3] xprt_write_space <-xs_write_space >>>> [4] rpc_task_sleep: task:43546@5 ... >>>> [5] xs_write_space <-xs_tcp_write_space >>>> >>>> [1] Task 43546 runs but is out of write space. >>>> >>>> [2] Space becomes available, xs_write_space() clears the >>>> SOCKWQ_ASYNC_NOSPACE bit. >>>> >>>> [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but >>>> this has not yet been queued and the wake up is lost. >>>> >>>> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >>>> which queues task 43546. >>>> >>>> [5] The call to sk->sk_write_space() at the end of xs_nospace() (which >>>> is supposed to handle the above race) does not call >>>> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >>>> thus the task is not woken. >>>> >>>> Fix the race by have xprt_wait_for_buffer_space() check for write >>>> space after putting the task to sleep. >>>> >>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >>>> --- >>>> include/linux/sunrpc/xprt.h | 1 + >>>> net/sunrpc/xprt.c | 4 ++++ >>>> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >>>> 3 files changed, 24 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h >>>> index a16070d..621e74b 100644 >>>> --- a/include/linux/sunrpc/xprt.h >>>> +++ b/include/linux/sunrpc/xprt.h >>>> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >>>> void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); >>>> void * (*buf_alloc)(struct rpc_task *task, size_t size); >>>> void (*buf_free)(void *buffer); >>>> + bool (*have_write_space)(struct rpc_xprt *task); >>>> int (*send_request)(struct rpc_task *task); >>>> void (*set_retrans_timeout)(struct rpc_task *task); >>>> void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >>>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >>>> index ea244b2..d3c1b1e 100644 >>>> --- a/net/sunrpc/xprt.c >>>> +++ b/net/sunrpc/xprt.c >>>> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) >>>> >>>> task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; >>>> rpc_sleep_on(&xprt->pending, task, action); >>>> + >>>> + /* Write space notification may race with putting task to sleep. */ >>>> + if (xprt->ops->have_write_space(xprt)) >>>> + rpc_wake_up_queued_task(&xprt->pending, task); >>>> } >>>> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >>>> >>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >>>> index bf16883..211de5b 100644 >>>> --- a/net/sunrpc/xprtsock.c >>>> +++ b/net/sunrpc/xprtsock.c >>>> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >>>> >>>> spin_unlock_bh(&xprt->transport_lock); >>>> >>>> - /* Race breaker in case memory is freed before above code is called */ >>>> - sk->sk_write_space(sk); >>>> return ret; >>>> } >>> >>> Instead of these callbacks, why not just add a call to >>> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in >>> xs_nospace()? Won’t that fix the existing race breaker? >> >> I don't see how that would help. If sk->sk_write_space was already >> called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to >> sk->sk_write_space will still be a nop. > > Sorry. Copy+paste error. I meant SOCKWQ_ASYNC_NOSPACE. > >> >> Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to set >> this bit when we don't know if there's space or not. > > Why not? I prefer my solution because: a) It obviously fixes the race (games with bits are less understandable). b) It requires fewer atomic ops. c) It doesn't require me to understand what the behaviour of the socket-internal SOCKWQ_ASYNC_NOSPACE bit is or should be. d) I'm not sure I understand the objection to the additional have_write_space method -- it has simple, clear behaviour. David ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls 2016-09-16 17:29 ` David Vrabel @ 2016-09-16 18:04 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 18:04 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 13:29, David Vrabel <david.vrabel@citrix.com> wrote: > > On 16/09/16 18:06, Trond Myklebust wrote: >> >>> On Sep 16, 2016, at 12:41, David Vrabel <david.vrabel@citrix.com> wrote: >>> >>> On 16/09/16 17:01, Trond Myklebust wrote: >>>> >>>>> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wrote: >>>>> >>>>> Write space becoming available may race with putting the task to sleep >>>>> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >>>>> race does not work. >>>>> >>>>> This (edited) partial trace illustrates the problem: >>>>> >>>>> [1] rpc_task_run_action: task:43546@5 ... action=call_transmit >>>>> [2] xs_write_space <-xs_tcp_write_space >>>>> [3] xprt_write_space <-xs_write_space >>>>> [4] rpc_task_sleep: task:43546@5 ... >>>>> [5] xs_write_space <-xs_tcp_write_space >>>>> >>>>> [1] Task 43546 runs but is out of write space. >>>>> >>>>> [2] Space becomes available, xs_write_space() clears the >>>>> SOCKWQ_ASYNC_NOSPACE bit. >>>>> >>>>> [3] xprt_write_space() attemts to wake xprt->snd_task (== 43546), but >>>>> this has not yet been queued and the wake up is lost. >>>>> >>>>> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >>>>> which queues task 43546. >>>>> >>>>> [5] The call to sk->sk_write_space() at the end of xs_nospace() (which >>>>> is supposed to handle the above race) does not call >>>>> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >>>>> thus the task is not woken. >>>>> >>>>> Fix the race by have xprt_wait_for_buffer_space() check for write >>>>> space after putting the task to sleep. >>>>> >>>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >>>>> --- >>>>> include/linux/sunrpc/xprt.h | 1 + >>>>> net/sunrpc/xprt.c | 4 ++++ >>>>> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >>>>> 3 files changed, 24 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.h >>>>> index a16070d..621e74b 100644 >>>>> --- a/include/linux/sunrpc/xprt.h >>>>> +++ b/include/linux/sunrpc/xprt.h >>>>> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >>>>> void (*connect)(struct rpc_xprt *xprt, struct rpc_task *task); >>>>> void * (*buf_alloc)(struct rpc_task *task, size_t size); >>>>> void (*buf_free)(void *buffer); >>>>> + bool (*have_write_space)(struct rpc_xprt *task); >>>>> int (*send_request)(struct rpc_task *task); >>>>> void (*set_retrans_timeout)(struct rpc_task *task); >>>>> void (*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >>>>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >>>>> index ea244b2..d3c1b1e 100644 >>>>> --- a/net/sunrpc/xprt.c >>>>> +++ b/net/sunrpc/xprt.c >>>>> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task *task, rpc_action action) >>>>> >>>>> task->tk_timeout = RPC_IS_SOFT(task) ? req->rq_timeout : 0; >>>>> rpc_sleep_on(&xprt->pending, task, action); >>>>> + >>>>> + /* Write space notification may race with putting task to sleep. */ >>>>> + if (xprt->ops->have_write_space(xprt)) >>>>> + rpc_wake_up_queued_task(&xprt->pending, task); >>>>> } >>>>> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >>>>> >>>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >>>>> index bf16883..211de5b 100644 >>>>> --- a/net/sunrpc/xprtsock.c >>>>> +++ b/net/sunrpc/xprtsock.c >>>>> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >>>>> >>>>> spin_unlock_bh(&xprt->transport_lock); >>>>> >>>>> - /* Race breaker in case memory is freed before above code is called */ >>>>> - sk->sk_write_space(sk); >>>>> return ret; >>>>> } >>>> >>>> Instead of these callbacks, why not just add a call to >>>> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in >>>> xs_nospace()? Won’t that fix the existing race breaker? >>> >>> I don't see how that would help. If sk->sk_write_space was already >>> called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to >>> sk->sk_write_space will still be a nop. >> >> Sorry. Copy+paste error. I meant SOCKWQ_ASYNC_NOSPACE. >> >>> >>> Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to set >>> this bit when we don't know if there's space or not. >> >> Why not? > > I prefer my solution because: > > a) It obviously fixes the race (games with bits are less understandable). > > b) It requires fewer atomic ops. > > c) It doesn't require me to understand what the behaviour of the > socket-internal SOCKWQ_ASYNC_NOSPACE bit is or should be. > > d) I'm not sure I understand the objection to the additional > have_write_space method -- it has simple, clear behaviour. > I don’t see the point of adding 24 lines of code over 3 different files if the problem can be solved with 1 line of code. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv1] sunrpc: fix write space race causing stalls @ 2016-09-16 18:04 ` Trond Myklebust 0 siblings, 0 replies; 9+ messages in thread From: Trond Myklebust @ 2016-09-16 18:04 UTC (permalink / raw) To: David Vrabel Cc: List Linux NFS Mailing, Schumaker Anna, List Linux Network Devel Mailing > On Sep 16, 2016, at 13:29, David Vrabel <david.vrabel@citrix.com> wrote: >=20 > On 16/09/16 18:06, Trond Myklebust wrote: >>=20 >>> On Sep 16, 2016, at 12:41, David Vrabel <david.vrabel@citrix.com> wrote= : >>>=20 >>> On 16/09/16 17:01, Trond Myklebust wrote: >>>>=20 >>>>> On Sep 16, 2016, at 08:28, David Vrabel <david.vrabel@citrix.com> wro= te: >>>>>=20 >>>>> Write space becoming available may race with putting the task to slee= p >>>>> in xprt_wait_for_buffer_space(). The existing mechanism to avoid the >>>>> race does not work. >>>>>=20 >>>>> This (edited) partial trace illustrates the problem: >>>>>=20 >>>>> [1] rpc_task_run_action: task:43546@5 ... action=3Dcall_transmit >>>>> [2] xs_write_space <-xs_tcp_write_space >>>>> [3] xprt_write_space <-xs_write_space >>>>> [4] rpc_task_sleep: task:43546@5 ... >>>>> [5] xs_write_space <-xs_tcp_write_space >>>>>=20 >>>>> [1] Task 43546 runs but is out of write space. >>>>>=20 >>>>> [2] Space becomes available, xs_write_space() clears the >>>>> SOCKWQ_ASYNC_NOSPACE bit. >>>>>=20 >>>>> [3] xprt_write_space() attemts to wake xprt->snd_task (=3D=3D 43546),= but >>>>> this has not yet been queued and the wake up is lost. >>>>>=20 >>>>> [4] xs_nospace() is called which calls xprt_wait_for_buffer_space() >>>>> which queues task 43546. >>>>>=20 >>>>> [5] The call to sk->sk_write_space() at the end of xs_nospace() (whic= h >>>>> is supposed to handle the above race) does not call >>>>> xprt_write_space() as the SOCKWQ_ASYNC_NOSPACE bit is clear and >>>>> thus the task is not woken. >>>>>=20 >>>>> Fix the race by have xprt_wait_for_buffer_space() check for write >>>>> space after putting the task to sleep. >>>>>=20 >>>>> Signed-off-by: David Vrabel <david.vrabel@citrix.com> >>>>> --- >>>>> include/linux/sunrpc/xprt.h | 1 + >>>>> net/sunrpc/xprt.c | 4 ++++ >>>>> net/sunrpc/xprtsock.c | 21 +++++++++++++++++++-- >>>>> 3 files changed, 24 insertions(+), 2 deletions(-) >>>>>=20 >>>>> diff --git a/include/linux/sunrpc/xprt.h b/include/linux/sunrpc/xprt.= h >>>>> index a16070d..621e74b 100644 >>>>> --- a/include/linux/sunrpc/xprt.h >>>>> +++ b/include/linux/sunrpc/xprt.h >>>>> @@ -129,6 +129,7 @@ struct rpc_xprt_ops { >>>>> =09void=09=09(*connect)(struct rpc_xprt *xprt, struct rpc_task *task)= ; >>>>> =09void *=09=09(*buf_alloc)(struct rpc_task *task, size_t size); >>>>> =09void=09=09(*buf_free)(void *buffer); >>>>> +=09bool (*have_write_space)(struct rpc_xprt *task); >>>>> =09int=09=09(*send_request)(struct rpc_task *task); >>>>> =09void=09=09(*set_retrans_timeout)(struct rpc_task *task); >>>>> =09void=09=09(*timer)(struct rpc_xprt *xprt, struct rpc_task *task); >>>>> diff --git a/net/sunrpc/xprt.c b/net/sunrpc/xprt.c >>>>> index ea244b2..d3c1b1e 100644 >>>>> --- a/net/sunrpc/xprt.c >>>>> +++ b/net/sunrpc/xprt.c >>>>> @@ -502,6 +502,10 @@ void xprt_wait_for_buffer_space(struct rpc_task = *task, rpc_action action) >>>>>=20 >>>>> =09task->tk_timeout =3D RPC_IS_SOFT(task) ? req->rq_timeout : 0; >>>>> =09rpc_sleep_on(&xprt->pending, task, action); >>>>> + >>>>> +=09/* Write space notification may race with putting task to sleep. = */ >>>>> +=09if (xprt->ops->have_write_space(xprt)) >>>>> +=09=09rpc_wake_up_queued_task(&xprt->pending, task); >>>>> } >>>>> EXPORT_SYMBOL_GPL(xprt_wait_for_buffer_space); >>>>>=20 >>>>> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c >>>>> index bf16883..211de5b 100644 >>>>> --- a/net/sunrpc/xprtsock.c >>>>> +++ b/net/sunrpc/xprtsock.c >>>>> @@ -472,8 +472,6 @@ static int xs_nospace(struct rpc_task *task) >>>>>=20 >>>>> =09spin_unlock_bh(&xprt->transport_lock); >>>>>=20 >>>>> -=09/* Race breaker in case memory is freed before above code is call= ed */ >>>>> -=09sk->sk_write_space(sk); >>>>> =09return ret; >>>>> } >>>>=20 >>>> Instead of these callbacks, why not just add a call to >>>> sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk) after queueing the task in >>>> xs_nospace()? Won=92t that fix the existing race breaker? >>>=20 >>> I don't see how that would help. If sk->sk_write_space was already >>> called, SOCKWQ_ASYNC_NOSPACE will still be clear and the next call to >>> sk->sk_write_space will still be a nop. >>=20 >> Sorry. Copy+paste error. I meant SOCKWQ_ASYNC_NOSPACE. >>=20 >>>=20 >>> Or did you mean SOCKWQ_ASYNC_NOSPACE here? It doesn't seem right to se= t >>> this bit when we don't know if there's space or not. >>=20 >> Why not? >=20 > I prefer my solution because: >=20 > a) It obviously fixes the race (games with bits are less understandable). >=20 > b) It requires fewer atomic ops. >=20 > c) It doesn't require me to understand what the behaviour of the > socket-internal SOCKWQ_ASYNC_NOSPACE bit is or should be. >=20 > d) I'm not sure I understand the objection to the additional > have_write_space method -- it has simple, clear behaviour. >=20 I don=92t see the point of adding 24 lines of code over 3 different files i= f the problem can be solved with 1 line of code. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-09-16 18:04 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-09-16 12:28 [PATCHv1] sunrpc: fix write space race causing stalls David Vrabel 2016-09-16 16:01 ` Trond Myklebust 2016-09-16 16:01 ` Trond Myklebust 2016-09-16 16:41 ` David Vrabel [not found] ` <57DC20C8.5000306-Sxgqhf6Nn4DQT0dZR+AlfA@public.gmane.org> 2016-09-16 17:06 ` Trond Myklebust 2016-09-16 17:06 ` Trond Myklebust 2016-09-16 17:29 ` David Vrabel 2016-09-16 18:04 ` Trond Myklebust 2016-09-16 18:04 ` Trond Myklebust
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.