From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 156901D7; Thu, 6 Dec 2018 05:25:04 +0100 (CET) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2980025ED8; Thu, 6 Dec 2018 04:25:03 +0000 (UTC) Received: from [10.72.12.143] (ovpn-12-143.pek2.redhat.com [10.72.12.143]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C8B37101F94C; Thu, 6 Dec 2018 04:24:53 +0000 (UTC) To: Ilya Maximets , Maxime Coquelin , dev@dpdk.org, jfreimann@redhat.com, tiwei.bie@intel.com, zhihong.wang@intel.com Cc: stable@dpdk.org, "Michael S. Tsirkin" References: <20181205094957.1938-3-maxime.coquelin@redhat.com> <16a4b2fd-c701-7822-3215-62e431e3f339@samsung.com> From: Jason Wang Message-ID: Date: Thu, 6 Dec 2018 12:24:48 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <16a4b2fd-c701-7822-3215-62e431e3f339@samsung.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Thu, 06 Dec 2018 04:25:03 +0000 (UTC) Subject: Re: [dpdk-dev] [2/5] vhost: enforce desc flags and content read ordering X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 06 Dec 2018 04:25:04 -0000 On 2018/12/5 下午9:33, Ilya Maximets wrote: > On 05.12.2018 12:49, Maxime Coquelin wrote: >> A read barrier is required to ensure that the ordering between >> descriptor's flags and content reads is enforced. >> >> Fixes: 2f3225a7d69b ("vhost: add vector filling support for packed ring") >> Cc: stable@dpdk.org >> >> Reported-by: Jason Wang >> Signed-off-by: Maxime Coquelin >> --- >> lib/librte_vhost/virtio_net.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c >> index f11ebb54f..68b72e7a5 100644 >> --- a/lib/librte_vhost/virtio_net.c >> +++ b/lib/librte_vhost/virtio_net.c >> @@ -520,6 +520,12 @@ fill_vec_buf_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, >> if (unlikely(!desc_is_avail(&descs[avail_idx], wrap_counter))) >> return -1; >> >> + /* >> + * The ordering between desc flags and desc >> + * content reads need to be enforced. >> + */ >> + rte_smp_rmb(); >> + > Same here. 'desc_is_avail' reads and uses the flags. i.e. > no way for reordering, > Writes must be ordered on the virtio side by the write barrier. > This means that if flags are updated (desc_is_avail() == true) > than the whole descriptor already updated and the data is written. > No need to have any read barriers here. In fact , the sequence might be: flag = read desc[avail_idx].flag [1] if(flag is not avail) {     read desc[avail_idx].id [2] } There's no data dependency but control dependency here, so 2 could be done before 1 without a rmb. Thanks > >> *desc_count = 0; >> *len = 0; >> >>