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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).