From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Network Development <netdev@vger.kernel.org>,
David Miller <davem@davemloft.net>,
rds-devel@oss.oracle.com,
Santosh Shilimkar <santosh.shilimkar@oracle.com>
Subject: Re: [PATCH net-next] RDS: deliver zerocopy completion notification with data as an optimization
Date: Wed, 21 Feb 2018 18:03:55 -0500 [thread overview]
Message-ID: <20180221230355.GH15244@oracle.com> (raw)
In-Reply-To: <CAF=yD-KuLHa=og50nw5MQpsN8dgCPJ+iFhiCVF=OvkVJFe8knA@mail.gmail.com>
On (02/21/18 17:50), Willem de Bruijn wrote:
>
> In the common case no more than one notification will be outstanding,
> but with a fixed number of notifications per packet, in edge cases this
> list may be long.
:
> Socket functions block if sk_err is non-zero. See for instance
> tcp_sendmsg_locked. It is set by most code that also calls
> sock_queue_err_skb and also on dequeue from err skb.
>
> This is the main reason that I would consider dropping error
> queue completely if you expect all users of RDS to use the
> cmsg on regular read to get these notifications.
I see. That's a good point, and maybe it makes sense to just have
a struct sk_buff_head rs_zcookie_quese on the rds_sock, and
have rds_rm_zerocopy_callback chain cookies ot this rs_zcookie_queue.
[discussion regarding rds_recvmsg return values elided]
> Okay. If callers must already handle 0 as a valid return code, then
> it is fine to add another case that does this.
>
> The extra branch in the hot path is still rather unfortunately. Could
> this be integrated in the existing if (nonblock) branch below?
that's where I first started. it got even hairier because the
callers expect a retval of 0 (-EAGAIN threw rds-stress into an
infinite loop of continulally trying to recv) and the end result
was just confusing code with the same number of branches..
let me revisit this when I spin out V2 without the sk_error_queue..
--Sowmini
next prev parent reply other threads:[~2018-02-21 23:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-02-21 20:19 [PATCH net-next] RDS: deliver zerocopy completion notification with data as an optimization Sowmini Varadhan
2018-02-21 21:04 ` Santosh Shilimkar
2018-02-21 21:54 ` Willem de Bruijn
2018-02-21 22:14 ` Sowmini Varadhan
2018-02-21 22:50 ` Willem de Bruijn
2018-02-21 23:03 ` Sowmini Varadhan [this message]
2018-02-21 23:45 ` Willem de Bruijn
2018-02-22 0:26 ` Sowmini Varadhan
2018-02-22 0:39 ` Willem de Bruijn
2018-02-22 13:36 ` Sowmini Varadhan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180221230355.GH15244@oracle.com \
--to=sowmini.varadhan@oracle.com \
--cc=davem@davemloft.net \
--cc=netdev@vger.kernel.org \
--cc=rds-devel@oss.oracle.com \
--cc=santosh.shilimkar@oracle.com \
--cc=willemdebruijn.kernel@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.