Re: write_space and software kTLS

[Chuck Lever III <chuck.lever@oracle.com>]

> On Mar 14, 2022, at 8:06 PM, Trond Myklebust <trondmy@hammerspace.com> wrote:
> 
> On Mon, 2022-03-14 at 21:35 +0000, Chuck Lever III wrote:
>> Hey Trond-
>> 
>> I've made some progress getting RPC-with-TLS working in
>> the Linux NFS client, but I recently hit an interesting
>> snag and could use a little advice.
>> 
>> The software kTLS infrastructure uses do_tcp_sendpages()
>> under the covers, and that function is clearing
>> SOCKWQ_ASYNC_NOSPACE from under xs_nospace(). That
>> prevents xs_run_error_worker() from waking up xprt->sending,
>> stalling an RPC transport waiting for more socket write
>> space. I'm not sure how to address this, and I'm interested
>> in your opinion.
>> 
> 
> How is it achieving this? We only set SOCKWQ_ASYNC_NOSPACE after the
> call to xprt_sock_sendmsg().

A kworker is clearing NOSPACE between the time xs_tcp_send_request()
sets it and the time xs_write_space() runs.

  kworker/u128:2-33    [003]   155.723869: rpc_socket_nospace:   task:000006cb@00000003 total=262380 remaining=131308
  kworker/u128:2-33    [003]   155.723870: bprint:               xs_nospace: sk=0xffff88810a8f0a00 setting SOCKWQ_ASYNC_NOSPACE
  kworker/u128:2-33    [003]   155.723879: xprt_transmit:        task:000006cb@00000003 xid=0x8ab69e2e seqno=0 status=-11
  kworker/u128:2-33    [003]   155.723881: xprt_release_xprt:    task:000006cc@00000003 snd_task:ffffffff
     kworker/3:2-116   [003]   155.723885: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
  kworker/u128:2-33    [003]   155.723888: rpc_task_run_action:  task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=-11 action=call_transmit_status
  kworker/u128:2-33    [003]   155.723889: rpc_task_run_action:  task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=0 action=call_transmit
  kworker/u128:2-33    [003]   155.723890: rpc_task_sleep:       task:000006cc@00000003 flags=ASYNC|MOVEABLE|NORTO|CRED_NOREF runstate=RUNNING|ACTIVE|NEED_XMIT|NEED_RECV status=-11 timeout=0 queue=xprt_sending
     kworker/1:2-115   [001]   155.733398: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
     kworker/1:2-115   [001]   155.733418: bprint:               do_tcp_sendpages: sk=0xffff88810a8f0a00 clearing SOCKWQ_ASYNC_NOSPACE
         openvpn-914   [001]   155.750263: bprint:               xs_write_space: sk=0xffff88810a8f0a00 SOCKWQ_ASYNC_NOSPACE was clear


>> For example, why check that flag rather than just waking
>> up xprt->sending unconditionally?
> 
> The socket code calls ->write_space() in all sorts of situations, so we
> need to distinguish between the cases where we are actually waiting for
> buffer memory, and the situations where we are not. Otherwise, we'd be
> calling xs_run_error_worker() all the time.

On my (admittedly limited) workloads, sk_stream_is_writeable()
does a good job of avoiding spurious wake-ups. However, to be
absolutely certain of our wake-up accounting, using a flag that
is local to the rpc_xprt and not overloaded might be wise?


--
Chuck Lever