From: Ilya Maximets <i.maximets@samsung.com>
To: Maxime Coquelin <maxime.coquelin@redhat.com>,
dev@dpdk.org, jfreimann@redhat.com, tiwei.bie@intel.com,
zhihong.wang@intel.com, jasowang@redhat.com
Subject: Re: [dpdk-dev] [5/5] vhost: remove useless casts to volatile
Date: Wed, 5 Dec 2018 16:52:30 +0300 [thread overview]
Message-ID: <1a4073a9-fc9b-a3c6-5761-7269fe955745@samsung.com> (raw)
In-Reply-To: <20181205094957.1938-6-maxime.coquelin@redhat.com>
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 <maxime.coquelin@redhat.com>
> ---
> 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;
>
>
>
next prev parent reply other threads:[~2018-12-05 13:52 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-12-05 9:49 [dpdk-dev] [PATCH 0/5] vhost: add missing barriers, remove useless volatiles Maxime Coquelin
2018-12-05 9:49 ` [dpdk-dev] [PATCH 1/5] vhost: enforce avail index and desc read ordering Maxime Coquelin
[not found] ` <CGME20181205113041eucas1p1943b9c13af2fb5b736ba4906b59a9cd5@eucas1p1.samsung.com>
2018-12-05 11:30 ` [dpdk-dev] [1/5] " Ilya Maximets
2018-12-06 4:17 ` Jason Wang
2018-12-06 12:48 ` Ilya Maximets
2018-12-06 13:25 ` Jason Wang
2018-12-06 13:48 ` Michael S. Tsirkin
2018-12-07 14:58 ` Ilya Maximets
2018-12-07 15:44 ` Michael S. Tsirkin
[not found] ` <CGME20181211103848eucas1p10c270ca8997fea8a2f55c2d94d02baea@eucas1p1.samsung.com>
2018-12-11 10:38 ` Ilya Maximets
2018-12-11 14:46 ` Maxime Coquelin
2018-12-05 9:49 ` [dpdk-dev] [PATCH 2/5] vhost: enforce desc flags and content " Maxime Coquelin
[not found] ` <CGME20181205133332eucas1p195b3864ed146403e314d7004d27be285@eucas1p1.samsung.com>
2018-12-05 13:33 ` [dpdk-dev] [2/5] " Ilya Maximets
2018-12-06 4:24 ` Jason Wang
2018-12-06 11:34 ` Ilya Maximets
2018-12-05 9:49 ` [dpdk-dev] [PATCH 3/5] vhost: prefetch descriptor after the read barrier Maxime Coquelin
2018-12-05 9:49 ` [dpdk-dev] [PATCH 4/5] vhost: remove useless prefetch for packed ring descriptor Maxime Coquelin
2018-12-05 9:49 ` [dpdk-dev] [PATCH 5/5] vhost: remove useless casts to volatile Maxime Coquelin
[not found] ` <CGME20181205135231eucas1p1c89281f6525a0fedab4a2fc0d2e21393@eucas1p1.samsung.com>
2018-12-05 13:52 ` Ilya Maximets [this message]
2018-12-06 16:59 ` [dpdk-dev] [5/5] " Maxime Coquelin
2018-12-07 11:16 ` Ilya Maximets
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=1a4073a9-fc9b-a3c6-5761-7269fe955745@samsung.com \
--to=i.maximets@samsung.com \
--cc=dev@dpdk.org \
--cc=jasowang@redhat.com \
--cc=jfreimann@redhat.com \
--cc=maxime.coquelin@redhat.com \
--cc=tiwei.bie@intel.com \
--cc=zhihong.wang@intel.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).