From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <bruce.richardson@intel.com>
Received: from mga02.intel.com (mga02.intel.com [134.134.136.20])
by dpdk.org (Postfix) with ESMTP id 79F97377C
for <dev@dpdk.org>; Wed, 25 May 2016 11:47:34 +0200 (CEST)
Received: from fmsmga003.fm.intel.com ([10.253.24.29])
by orsmga101.jf.intel.com with ESMTP; 25 May 2016 02:47:33 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.26,363,1459839600"; d="scan'208";a="708628023"
Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.220.175])
by FMSMGA003.fm.intel.com with SMTP; 25 May 2016 02:47:30 -0700
Received: by (sSMTP sendmail emulation); Wed, 25 May 2016 10:47:30 +0025
Date: Wed, 25 May 2016 10:47:30 +0100
From: Bruce Richardson <bruce.richardson@intel.com>
To: "Michael S. Tsirkin" <mst@redhat.com>
Cc: "Xie, Huawei" <huawei.xie@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"thomas.monjalon@6wind.com" <thomas.monjalon@6wind.com>,
Yuanhan Liu <yuanhan.liu@linux.intel.com>,
"Tan, Jianfeng" <jianfeng.tan@intel.com>
Message-ID: <20160525094729.GA4256@bricha3-MOBL3>
References: <1464106601-981-1-git-send-email-huawei.xie@intel.com>
<C37D651A908B024F974696C65296B57B4C7A248F@SHSMSX101.ccr.corp.intel.com>
<20160525113224-mutt-send-email-mst@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20160525113224-mutt-send-email-mst@redhat.com>
Organization: Intel Shannon Ltd.
User-Agent: Mutt/1.5.23 (2014-03-12)
Subject: Re: [dpdk-dev] [PATCH] virtio: use volatile to get used->idx in the
loop
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 25 May 2016 09:47:34 -0000
On Wed, May 25, 2016 at 11:34:24AM +0300, Michael S. Tsirkin wrote:
> On Wed, May 25, 2016 at 08:25:20AM +0000, Xie, Huawei wrote:
> > On 5/25/2016 4:12 PM, Xie, Huawei wrote:
> > > There is no external function call or any barrier in the loop,
> > > the used->idx would only be retrieved once.
> > >
> > > Signed-off-by: Huawei Xie <huawei.xie@intel.com>
> > > ---
> > > drivers/net/virtio/virtio_ethdev.c | 3 ++-
> > > 1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> > > index c3fb628..f6d6305 100644
> > > --- a/drivers/net/virtio/virtio_ethdev.c
> > > +++ b/drivers/net/virtio/virtio_ethdev.c
> > > @@ -204,7 +204,8 @@ virtio_send_command(struct virtqueue *vq, struct virtio_pmd_ctrl *ctrl,
> > > usleep(100);
> > > }
> > >
> > > - while (vq->vq_used_cons_idx != vq->vq_ring.used->idx) {
> > > + while (vq->vq_used_cons_idx !=
> > > + *((volatile uint16_t *)(&vq->vq_ring.used->idx))) {
> > > uint32_t idx, desc_idx, used_idx;
> > > struct vring_used_elem *uep;
> > >
> >
> > Find this issue when do the code rework of RX/TX queue.
> > As in other places, we also have loop retrieving the value of avial->idx
> > or used->idx, i prefer to declare the index in vq structure as volatile
> > to avoid potential issue.
Is there a reason why the value is not always volatile? I would have thought
it would be generally safer to mark the actual value as volatile inside the
structure definition itself? In any cases where we do want to store the value
locally and not re-access the structure, a local variable can be used.
Regards,
/Bruce
>
> It might be a good idea to wrap this in a macro
> similar to ACCESS_ONCE in Linux.
>
> >
> > Stephen:
> > Another question is why we need a loop here?
> >
> > /huawei
>
> --
> MST