Skip to content

Commit 71c0b9a

Browse files
committed
vhost: Remove redundant use of read_barrier_depends() barrier
Since commit 76ebbe7 ("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 <[email protected]> Acked-by: Peter Zijlstra (Intel) <[email protected]> Suggested-by: Jason Wang <[email protected]> Acked-by: Paul E. McKenney <[email protected]> Signed-off-by: Will Deacon <[email protected]>
1 parent 002dff3 commit 71c0b9a

File tree

1 file changed

+0
-5
lines changed

1 file changed

+0
-5
lines changed

drivers/vhost/vhost.c

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2092,11 +2092,6 @@ static int get_indirect(struct vhost_virtqueue *vq,
20922092
return ret;
20932093
}
20942094
iov_iter_init(&from, READ, vq->indirect, ret, len);
2095-
2096-
/* We will use the result as an address to read from, so most
2097-
* architectures only need a compiler barrier here. */
2098-
read_barrier_depends();
2099-
21002095
count = len / sizeof desc;
21012096
/* Buffers are chained via a 16 bit next field, so
21022097
* we can have at most 2^16 of these. */

0 commit comments

Comments
 (0)