summaryrefslogtreecommitdiffstats
path: root/drivers/vhost
diff options
context:
space:
mode:
authorWill Deacon <will@kernel.org>2019-10-30 16:22:17 +0000
committerWill Deacon <will@kernel.org>2020-07-21 10:50:36 +0100
commit71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2 (patch)
treed76403db6d40c7e6a7df401ab3f3545185716933 /drivers/vhost
parent002dff36acfba3476b685a09f78ffb7c452f5951 (diff)
downloadlinux-71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2.tar.gz
linux-71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2.tar.bz2
linux-71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2.zip
vhost: Remove redundant use of read_barrier_depends() barrier
Since commit 76ebbe78f739 ("locking/barriers: Add implicit smp_read_barrier_depends() to READ_ONCE()"), there is no need to use smp_read_barrier_depends() outside of the Alpha architecture code. Unfortunately, there is precisely _one_ user in the vhost code, and there isn't an obvious READ_ONCE() access making the barrier redundant. However, on closer inspection (thanks, Jason), it appears that vring synchronisation between the producer and consumer occurs via the 'avail_idx' field, which is followed up by an rmb() in vhost_get_vq_desc(), making the read_barrier_depends() redundant on Alpha. Jason says: | I'm also confused about the barrier here, basically in driver side | we did: | | 1) allocate pages | 2) store pages in indirect->addr | 3) smp_wmb() | 4) increase the avail idx (somehow a tail pointer of vring) | | in vhost we did: | | 1) read avail idx | 2) smp_rmb() | 3) read indirect->addr | 4) read from indirect->addr | | It looks to me even the data dependency barrier is not necessary | since we have rmb() which is sufficient for us to the correct | indirect->addr and driver are not expected to do any writing to | indirect->addr after avail idx is increased Remove the redundant barrier invocation. Acked-by: Michael S. Tsirkin <mst@redhat.com> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> Suggested-by: Jason Wang <jasowang@redhat.com> Acked-by: Paul E. McKenney <paulmck@kernel.org> Signed-off-by: Will Deacon <will@kernel.org>
Diffstat (limited to 'drivers/vhost')
-rw-r--r--drivers/vhost/vhost.c5
1 files changed, 0 insertions, 5 deletions
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index d7b8df3edffc..74d135ee7e26 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2092,11 +2092,6 @@ static int get_indirect(struct vhost_virtqueue *vq,
return ret;
}
iov_iter_init(&from, READ, vq->indirect, ret, len);
-
- /* We will use the result as an address to read from, so most
- * architectures only need a compiler barrier here. */
- read_barrier_depends();
-
count = len / sizeof desc;
/* Buffers are chained via a 16 bit next field, so
* we can have at most 2^16 of these. */