From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailout1.w1.samsung.com (mailout1.w1.samsung.com [210.118.77.11]) by dpdk.org (Postfix) with ESMTP id 81B0F1B141 for ; Wed, 5 Dec 2018 14:52:34 +0100 (CET) Received: from eucas1p2.samsung.com (unknown [182.198.249.207]) by mailout1.w1.samsung.com (KnoxPortal) with ESMTP id 20181205135233euoutp01b392a78b809d3798e1913097c0537998~tdCXarKB60850508505euoutp01I for ; Wed, 5 Dec 2018 13:52:33 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 mailout1.w1.samsung.com 20181205135233euoutp01b392a78b809d3798e1913097c0537998~tdCXarKB60850508505euoutp01I DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=samsung.com; s=mail20170921; t=1544017953; bh=WpJRfAokETxjNueznVoZs/ztJslsnyynMvv4HnqnXao=; h=Subject:To:From:Date:In-Reply-To:References:From; b=fYVl3AvDA+6h7/qac77JMvKB5dCmCXKmyAvmweY1s0R2TySBoDWAe0UHH1qVwugOP g92nBV26JsmoXaMkxZ5fODoTG5C5RsiGPzvqGgSOuWZ7G4D93fkIcOJGrnHE4CxcvJ 6eB0GKRBVgzCJq9eeWJ7HxEksTsOtJPyIuUAlw2c= Received: from eusmges2new.samsung.com (unknown [203.254.199.244]) by eucas1p1.samsung.com (KnoxPortal) with ESMTP id 20181205135232eucas1p16755dcd8a77ddad326fc55e54eed73c4~tdCXA8FVy0946509465eucas1p1X; Wed, 5 Dec 2018 13:52:32 +0000 (GMT) Received: from eucas1p1.samsung.com ( [182.198.249.206]) by eusmges2new.samsung.com (EUCPMTA) with SMTP id 83.B3.04294.028D70C5; Wed, 5 Dec 2018 13:52:32 +0000 (GMT) Received: from eusmtrp1.samsung.com (unknown [182.198.249.138]) by eucas1p1.samsung.com (KnoxPortal) with ESMTPA id 20181205135231eucas1p1c89281f6525a0fedab4a2fc0d2e21393~tdCWI366W2970329703eucas1p1A; Wed, 5 Dec 2018 13:52:31 +0000 (GMT) Received: from eusmgms1.samsung.com (unknown [182.198.249.179]) by eusmtrp1.samsung.com (KnoxPortal) with ESMTP id 20181205135231eusmtrp1197cbc44c3aaa69b7079206e49e41aa0~tdCV5g_XQ0992809928eusmtrp1r; Wed, 5 Dec 2018 13:52:31 +0000 (GMT) X-AuditID: cbfec7f4-84fff700000010c6-0a-5c07d820b31f Received: from eusmtip1.samsung.com ( [203.254.199.221]) by eusmgms1.samsung.com (EUCPMTA) with SMTP id 19.E4.04284.F18D70C5; Wed, 5 Dec 2018 13:52:31 +0000 (GMT) Received: from [106.109.129.180] (unknown [106.109.129.180]) by eusmtip1.samsung.com (KnoxPortal) with ESMTPA id 20181205135231eusmtip1ef9dc1144e33168fa8af752d23140890~tdCVcJeX70596605966eusmtip1G; Wed, 5 Dec 2018 13:52:31 +0000 (GMT) To: Maxime Coquelin , dev@dpdk.org, jfreimann@redhat.com, tiwei.bie@intel.com, zhihong.wang@intel.com, jasowang@redhat.com From: Ilya Maximets Message-ID: <1a4073a9-fc9b-a3c6-5761-7269fe955745@samsung.com> Date: Wed, 5 Dec 2018 16:52:30 +0300 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: <20181205094957.1938-6-maxime.coquelin@redhat.com> Content-Language: en-GB Content-Transfer-Encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrOKsWRmVeSWpSXmKPExsWy7djPc7oKN9hjDK6sE7N492k7k8WV9p/s FssufWayOLdmKYvFsc49LBZbG/4zWWy+OInJgd3j14KlrB6L97xk8ni/7yqbR9+WVYwBLFFc NimpOZllqUX6dglcGefmGRbMEan4NPUDYwPjPoEuRk4OCQETia6TO9m6GLk4hARWMEr8m36M BcL5wijx4N5WdgjnM6PEoh197DAtO6YdYoZILGeUOH/7MJTzkVHi6ul5jCBVwgJWEhO+fQRL iAjMYpRoej2TDSTBJqAjcWr1EbAiXgE7icWLJ4LZLAIqEs031rKA2KICERId91ezQdQISpyc +QQszilgLzF3Xz+YzSwgLtH0ZSUrhC0vsf3tHLBlEgLT2SXaejeyQtzqIvH9QRsThC0s8er4 FqgfZCROT+5hgbDrJe63vGSEaO5glJh+6B9Ug73EltfngBo4gDZoSqzfpQ8RdpTYMPUOG0hY QoBP4sZbQYgb+CQmbZvODBHmlehoE4KoVpH4fXA5M4QtJXHz3WeoCzwkPu5dyzKBUXEWki9n IflsFpLPZiHcsICRZRWjeGppcW56arFRXmq5XnFibnFpXrpecn7uJkZg8jn97/iXHYy7/iQd YhTgYFTi4ZWYwhYjxJpYVlyZe4hRgoNZSYR3hQ17jBBvSmJlVWpRfnxRaU5q8SFGaQ4WJXHe aoYH0UIC6YklqdmpqQWpRTBZJg5OqQZG403Tjja8m/uUcX2n4ebpqTtXFD2Q0vpmvHdC8Ouy D5sadzu9KkkKXajoyp5jp9l4529wQZOrndAngYWvPMN2tbVkJ2yUP/l2QyBbtmvINeHwmsUH VpyRfFTS4LH0DfeKaT/7vbIvdLsFvbCUYz0ncT1dcTIDv3vjVcEPjYJn1d6p+Tt7931UYinO SDTUYi4qTgQA0nUDajoDAAA= X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFlrLIsWRmVeSWpSXmKPExsVy+t/xu7ryN9hjDJ4dk7d492k7k8WV9p/s FssufWayOLdmKYvFsc49LBZbG/4zWWy+OInJgd3j14KlrB6L97xk8ni/7yqbR9+WVYwBLFF6 NkX5pSWpChn5xSW2StGGFkZ6hpYWekYmlnqGxuaxVkamSvp2NimpOZllqUX6dgl6GefmGRbM Ean4NPUDYwPjPoEuRk4OCQETiR3TDjF3MXJxCAksZZT4cfwSK0RCSuLHrwtQtrDEn2tdbBBF 7xklfm19yQ6SEBawkpjw7SNYt4jALEaJlSdAqjiBquwkVvdcBOtmE9CROLX6CCOIzQsUX7x4 IpjNIqAi0XxjLQuILSoQIXH25TqoGkGJkzOfgMU5Bewl5u7rB7OZBdQl/sy7xAxhi0s0fVnJ CmHLS2x/O4d5AqPgLCTts5C0zELSMgtJywJGllWMIqmlxbnpucWGesWJucWleel6yfm5mxiB UbXt2M/NOxgvbQw+xCjAwajEwys5hS1GiDWxrLgy9xCjBAezkgjvChv2GCHelMTKqtSi/Pii 0pzU4kOMpkDPTWSWEk3OB0Z8Xkm8oamhuYWlobmxubGZhZI473mDyighgfTEktTs1NSC1CKY PiYOTqkGRl5jowmpef9vJoeqrWLc+eCD99cfO6/aaG7ksGo5fuWIodHHHX+1pBwzUickNVXx 7XjkELJV9vEb7+rw7WczQvf7rPX7P7F1vVDQ9JDaw5nLj89/uXzDe2fvsiK9RpkjeU2ssQ93 ajR0Ca8XKVUW1CheU92wLdXHaGfzwy+8cg7P+Wc7ui6bocRSnJFoqMVcVJwIAH7JUTvAAgAA X-CMS-MailID: 20181205135231eucas1p1c89281f6525a0fedab4a2fc0d2e21393 X-Msg-Generator: CA Content-Type: text/plain; charset="utf-8" X-RootMTR: 20181205135231eucas1p1c89281f6525a0fedab4a2fc0d2e21393 X-EPHeader: CA CMS-TYPE: 201P X-CMS-RootMailID: 20181205135231eucas1p1c89281f6525a0fedab4a2fc0d2e21393 References: <20181205094957.1938-6-maxime.coquelin@redhat.com> Subject: Re: [dpdk-dev] [5/5] vhost: remove useless casts to volatile 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: Wed, 05 Dec 2018 13:52:34 -0000 On 05.12.2018 12:49, Maxime Coquelin wrote: > Cast to volatile is done when reading avail index and writing > the used index. This would not be necessary if proper barriers > are used. 'volatile' and barriers are not really connected. 'volatile' is the disabling of the compiler optimizations, while barriers are for runtime CPU level optimizations. In general, casts here made to force compiler to actually read the value and not cache it somehow. In fact that vhost library never writes to avail index, "very smart" compiler could drop it at all. None of modern compilers will do that for a single operation within a function, so, volatiles are not really necessary in current code, but they could save some nerves in case of code/compiler changes. OTOH, IMHO, the main purpose of the casts in current code is the self-documenting. Casts forces to pay special attention to these variables and reminds that they could be updated in other process. Casts allows to understand which variables are local and which are shared. I don't think that we should remove them anyway. > > Now that the read barrier has been added, we can remove these > cast to volatile. > > Signed-off-by: Maxime Coquelin > --- > lib/librte_vhost/virtio_net.c | 7 +++---- > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index 679ce388b..eab1a5b4c 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -114,7 +114,7 @@ flush_shadow_used_ring_split(struct virtio_net *dev, struct vhost_virtqueue *vq) > > vhost_log_cache_sync(dev, vq); > > - *(volatile uint16_t *)&vq->used->idx += vq->shadow_used_idx; > + vq->used->idx += vq->shadow_used_idx; > vq->shadow_used_idx = 0; > vhost_log_used_vring(dev, vq, offsetof(struct vring_used, idx), > sizeof(vq->used->idx)); > @@ -794,7 +794,7 @@ virtio_dev_rx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, > struct buf_vector buf_vec[BUF_VECTOR_MAX]; > uint16_t avail_head; > > - avail_head = *((volatile uint16_t *)&vq->avail->idx); > + avail_head = vq->avail->idx; > > /* > * The ordering between avail index and > @@ -1379,8 +1379,7 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, > } > } > > - free_entries = *((volatile uint16_t *)&vq->avail->idx) - > - vq->last_avail_idx; > + free_entries = vq->avail->idx - vq->last_avail_idx; > if (free_entries == 0) > return 0; > > >