diff options
| author |   Will Deacon <will@kernel.org> | 2019-10-30 16:22:17 +0000 |
|---|---|---|
| committer | Will Deacon <will@kernel.org> | 2020-07-21 10:50:36 +0100 |
| commit | 71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2 (patch) | |
| tree | d76403db6d40c7e6a7df401ab3f3545185716933 /drivers/vhost/vhost.c | |
| parent | asm/rwonce: Don't pull <asm/barrier.h> into 'asm-generic/rwonce.h' (diff) | |
| download | linux-dev-71c0b9a65cefa8c34eab83d337a1e3ec61fb7cc2.tar.xz linux-dev-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/vhost.c')
| -rw-r--r-- | drivers/vhost/vhost.c | 5 |
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. */ |