All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sowmini Varadhan <sowmini.varadhan@oracle.com>
To: David Miller <davem@davemloft.net>
Cc: david.stevens@oracle.com, netdev@vger.kernel.org
Subject: Re: [PATCH net-next v2 2/2] Re-check for a VIO_DESC_READY data descriptor after short udelay()
Date: Sat, 6 Sep 2014 17:02:53 -0400	[thread overview]
Message-ID: <20140906210253.GA5710@oracle.com> (raw)
In-Reply-To: <20140905134758.GB1256@oracle.com>

On (09/05/14 09:47), Sowmini Varadhan wrote:
> > The memory barrier exists in order to make sure the cookies et al. are
> > globally visible before the VIO_DESC_READY.  We don't want stores to
> > be reordered such that the VIO_DESC_READY is seen too early.
> 
> Ok, though David (dls) was just pointing out that a rmb() might
> be missing in vnet_walk_rx_one() before checking for READY descriptor

Stared at this a bit over the last two days, checked
the documentation, discussed with dls offline - looks like 
(a) the rmb() thing was mostly a red-herring/fud 
(b) we do need the wmb()

The wmb() part is working correctly as designed:

The producer will do
  /* code to set up cookies */
  wmb(); /* makes sure above changes are committed */
  d->hdr.state = VIO_DESC_READY;

the consumer will do

        if (desc->hdr.state != VIO_DESC_READY)
                return 1;
        err = vnet_rx_one(port, desc->size, desc->cookies, desc->ncookies);
                :
        desc->hdr.state = VIO_DESC_DONE;

So the vnet_rx_one() will only use valid cookie information at
all times.

This allows the code to correctly able to read multiple READY descriptors 
for a single LDC trigger, which it already does today.
(and it would be needlessly inefficient to clamp this down to
only one descriptor read per LDC-start in the vnet_rx())

So what (if any) is the outstanding question about wmb() at this 
point?

--Sowmini

  reply	other threads:[~2014-09-06 21:03 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-02 16:20 [PATCH net-next v2 2/2] Re-check for a VIO_DESC_READY data descriptor after short udelay() Sowmini Varadhan
2014-09-02 16:27 ` David L Stevens
2014-09-02 16:32   ` Sowmini Varadhan
2014-09-05  5:36   ` David Miller
2014-09-05 13:47     ` Sowmini Varadhan
2014-09-06 21:02       ` Sowmini Varadhan [this message]
     [not found]         ` <20140907181510.GA23753@oracle.com>
2014-09-07 19:36           ` Raghuram Kothakota
2014-09-07 23:19         ` David Miller
2014-09-08 13:45         ` David L Stevens
2014-09-08 14:03           ` 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=20140906210253.GA5710@oracle.com \
    --to=sowmini.varadhan@oracle.com \
    --cc=davem@davemloft.net \
    --cc=david.stevens@oracle.com \
    --cc=netdev@vger.kernel.org \
    /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.