netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
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

  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).