Skip to content

Commit ea16c51

Browse files
mstsirkinmstsirkin
committed
vhost: move acked_features to VQs
Refactor code to make sure features are only accessed under VQ mutex. This makes everything simpler, no need for RCU here anymore. Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
1 parent 98f9ca0 commit ea16c51

File tree

5 files changed

+44
-42
lines changed

5 files changed

+44
-42
lines changed

‎drivers/vhost/net.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -585,9 +585,9 @@ static void handle_rx(struct vhost_net *net)
585585
vhost_hlen = nvq->vhost_hlen;
586586
sock_hlen = nvq->sock_hlen;
587587

588-
vq_log = unlikely(vhost_has_feature(&net->dev, VHOST_F_LOG_ALL)) ?
588+
vq_log = unlikely(vhost_has_feature(vq, VHOST_F_LOG_ALL)) ?
589589
vq->log : NULL;
590-
mergeable = vhost_has_feature(&net->dev, VIRTIO_NET_F_MRG_RXBUF);
590+
mergeable = vhost_has_feature(vq, VIRTIO_NET_F_MRG_RXBUF);
591591

592592
while ((sock_len = peek_head_len(sock->sk))) {
593593
sock_len += sock_hlen;
@@ -1051,15 +1051,13 @@ static int vhost_net_set_features(struct vhost_net *n, u64 features)
10511051
mutex_unlock(&n->dev.mutex);
10521052
return -EFAULT;
10531053
}
1054-
n->dev.acked_features = features;
1055-
smp_wmb();
10561054
for (i = 0; i < VHOST_NET_VQ_MAX; ++i) {
10571055
mutex_lock(&n->vqs[i].vq.mutex);
1056+
n->vqs[i].vq.acked_features = features;
10581057
n->vqs[i].vhost_hlen = vhost_hlen;
10591058
n->vqs[i].sock_hlen = sock_hlen;
10601059
mutex_unlock(&n->vqs[i].vq.mutex);
10611060
}
1062-
vhost_net_flush(n);
10631061
mutex_unlock(&n->dev.mutex);
10641062
return 0;
10651063
}

‎drivers/vhost/scsi.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1373,6 +1373,9 @@ vhost_scsi_clear_endpoint(struct vhost_scsi *vs,
13731373

13741374
static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
13751375
{
1376+
struct vhost_virtqueue *vq;
1377+
int i;
1378+
13761379
if (features & ~VHOST_SCSI_FEATURES)
13771380
return -EOPNOTSUPP;
13781381

@@ -1382,9 +1385,13 @@ static int vhost_scsi_set_features(struct vhost_scsi *vs, u64 features)
13821385
mutex_unlock(&vs->dev.mutex);
13831386
return -EFAULT;
13841387
}
1385-
vs->dev.acked_features = features;
1386-
smp_wmb();
1387-
vhost_scsi_flush(vs);
1388+
1389+
for (i = 0; i < VHOST_SCSI_MAX_VQ; i++) {
1390+
vq = &vs->vqs[i].vq;
1391+
mutex_lock(&vq->mutex);
1392+
vq->acked_features = features;
1393+
mutex_unlock(&vq->mutex);
1394+
}
13881395
mutex_unlock(&vs->dev.mutex);
13891396
return 0;
13901397
}
@@ -1591,10 +1598,6 @@ tcm_vhost_do_plug(struct tcm_vhost_tpg *tpg,
15911598
return;
15921599

15931600
mutex_lock(&vs->dev.mutex);
1594-
if (!vhost_has_feature(&vs->dev, VIRTIO_SCSI_F_HOTPLUG)) {
1595-
mutex_unlock(&vs->dev.mutex);
1596-
return;
1597-
}
15981601

15991602
if (plug)
16001603
reason = VIRTIO_SCSI_EVT_RESET_RESCAN;
@@ -1603,8 +1606,9 @@ tcm_vhost_do_plug(struct tcm_vhost_tpg *tpg,
16031606

16041607
vq = &vs->vqs[VHOST_SCSI_VQ_EVT].vq;
16051608
mutex_lock(&vq->mutex);
1606-
tcm_vhost_send_evt(vs, tpg, lun,
1607-
VIRTIO_SCSI_T_TRANSPORT_RESET, reason);
1609+
if (vhost_has_feature(vq, VIRTIO_SCSI_F_HOTPLUG))
1610+
tcm_vhost_send_evt(vs, tpg, lun,
1611+
VIRTIO_SCSI_T_TRANSPORT_RESET, reason);
16081612
mutex_unlock(&vq->mutex);
16091613
mutex_unlock(&vs->dev.mutex);
16101614
}

‎drivers/vhost/test.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,15 +241,18 @@ static long vhost_test_reset_owner(struct vhost_test *n)
241241

242242
static int vhost_test_set_features(struct vhost_test *n, u64 features)
243243
{
244+
struct vhost_virtqueue *vq;
245+
244246
mutex_lock(&n->dev.mutex);
245247
if ((features & (1 << VHOST_F_LOG_ALL)) &&
246248
!vhost_log_access_ok(&n->dev)) {
247249
mutex_unlock(&n->dev.mutex);
248250
return -EFAULT;
249251
}
250-
n->dev.acked_features = features;
251-
smp_wmb();
252-
vhost_test_flush(n);
252+
vq = &n->vqs[VHOST_TEST_VQ];
253+
mutex_lock(&vq->mutex);
254+
vq->acked_features = features;
255+
mutex_unlock(&vq->mutex);
253256
mutex_unlock(&n->dev.mutex);
254257
return 0;
255258
}

‎drivers/vhost/vhost.c

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,7 @@ static void vhost_vq_reset(struct vhost_dev *dev,
191191
vq->log_used = false;
192192
vq->log_addr = -1ull;
193193
vq->private_data = NULL;
194+
vq->acked_features = 0;
194195
vq->log_base = NULL;
195196
vq->error_ctx = NULL;
196197
vq->error = NULL;
@@ -524,11 +525,13 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
524525

525526
for (i = 0; i < d->nvqs; ++i) {
526527
int ok;
528+
bool log;
529+
527530
mutex_lock(&d->vqs[i]->mutex);
531+
log = log_all || vhost_has_feature(d->vqs[i], VHOST_F_LOG_ALL);
528532
/* If ring is inactive, will check when it's enabled. */
529533
if (d->vqs[i]->private_data)
530-
ok = vq_memory_access_ok(d->vqs[i]->log_base, mem,
531-
log_all);
534+
ok = vq_memory_access_ok(d->vqs[i]->log_base, mem, log);
532535
else
533536
ok = 1;
534537
mutex_unlock(&d->vqs[i]->mutex);
@@ -538,12 +541,12 @@ static int memory_access_ok(struct vhost_dev *d, struct vhost_memory *mem,
538541
return 1;
539542
}
540543

541-
static int vq_access_ok(struct vhost_dev *d, unsigned int num,
544+
static int vq_access_ok(struct vhost_virtqueue *vq, unsigned int num,
542545
struct vring_desc __user *desc,
543546
struct vring_avail __user *avail,
544547
struct vring_used __user *used)
545548
{
546-
size_t s = vhost_has_feature(d, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
549+
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
547550
return access_ok(VERIFY_READ, desc, num * sizeof *desc) &&
548551
access_ok(VERIFY_READ, avail,
549552
sizeof *avail + num * sizeof *avail->ring + s) &&
@@ -565,16 +568,16 @@ EXPORT_SYMBOL_GPL(vhost_log_access_ok);
565568

566569
/* Verify access for write logging. */
567570
/* Caller should have vq mutex and device mutex */
568-
static int vq_log_access_ok(struct vhost_dev *d, struct vhost_virtqueue *vq,
571+
static int vq_log_access_ok(struct vhost_virtqueue *vq,
569572
void __user *log_base)
570573
{
571574
struct vhost_memory *mp;
572-
size_t s = vhost_has_feature(d, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
575+
size_t s = vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX) ? 2 : 0;
573576

574577
mp = rcu_dereference_protected(vq->dev->memory,
575578
lockdep_is_held(&vq->mutex));
576579
return vq_memory_access_ok(log_base, mp,
577-
vhost_has_feature(vq->dev, VHOST_F_LOG_ALL)) &&
580+
vhost_has_feature(vq, VHOST_F_LOG_ALL)) &&
578581
(!vq->log_used || log_access_ok(log_base, vq->log_addr,
579582
sizeof *vq->used +
580583
vq->num * sizeof *vq->used->ring + s));
@@ -584,8 +587,8 @@ static int vq_log_access_ok(struct vhost_dev *d, struct vhost_virtqueue *vq,
584587
/* Caller should have vq mutex and device mutex */
585588
int vhost_vq_access_ok(struct vhost_virtqueue *vq)
586589
{
587-
return vq_access_ok(vq->dev, vq->num, vq->desc, vq->avail, vq->used) &&
588-
vq_log_access_ok(vq->dev, vq, vq->log_base);
590+
return vq_access_ok(vq, vq->num, vq->desc, vq->avail, vq->used) &&
591+
vq_log_access_ok(vq, vq->log_base);
589592
}
590593
EXPORT_SYMBOL_GPL(vhost_vq_access_ok);
591594

@@ -612,8 +615,7 @@ static long vhost_set_memory(struct vhost_dev *d, struct vhost_memory __user *m)
612615
return -EFAULT;
613616
}
614617

615-
if (!memory_access_ok(d, newmem,
616-
vhost_has_feature(d, VHOST_F_LOG_ALL))) {
618+
if (!memory_access_ok(d, newmem, 0)) {
617619
kfree(newmem);
618620
return -EFAULT;
619621
}
@@ -726,7 +728,7 @@ long vhost_vring_ioctl(struct vhost_dev *d, int ioctl, void __user *argp)
726728
* If it is not, we don't as size might not have been setup.
727729
* We will verify when backend is configured. */
728730
if (vq->private_data) {
729-
if (!vq_access_ok(d, vq->num,
731+
if (!vq_access_ok(vq, vq->num,
730732
(void __user *)(unsigned long)a.desc_user_addr,
731733
(void __user *)(unsigned long)a.avail_user_addr,
732734
(void __user *)(unsigned long)a.used_user_addr)) {
@@ -866,7 +868,7 @@ long vhost_dev_ioctl(struct vhost_dev *d, unsigned int ioctl, void __user *argp)
866868
vq = d->vqs[i];
867869
mutex_lock(&vq->mutex);
868870
/* If ring is inactive, will check when it's enabled. */
869-
if (vq->private_data && !vq_log_access_ok(d, vq, base))
871+
if (vq->private_data && !vq_log_access_ok(vq, base))
870872
r = -EFAULT;
871873
else
872874
vq->log_base = base;
@@ -1434,11 +1436,11 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
14341436
* interrupts. */
14351437
smp_mb();
14361438

1437-
if (vhost_has_feature(dev, VIRTIO_F_NOTIFY_ON_EMPTY) &&
1439+
if (vhost_has_feature(vq, VIRTIO_F_NOTIFY_ON_EMPTY) &&
14381440
unlikely(vq->avail_idx == vq->last_avail_idx))
14391441
return true;
14401442

1441-
if (!vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) {
1443+
if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
14421444
__u16 flags;
14431445
if (__get_user(flags, &vq->avail->flags)) {
14441446
vq_err(vq, "Failed to get flags");
@@ -1499,7 +1501,7 @@ bool vhost_enable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
14991501
if (!(vq->used_flags & VRING_USED_F_NO_NOTIFY))
15001502
return false;
15011503
vq->used_flags &= ~VRING_USED_F_NO_NOTIFY;
1502-
if (!vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) {
1504+
if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
15031505
r = vhost_update_used_flags(vq);
15041506
if (r) {
15051507
vq_err(vq, "Failed to enable notification at %p: %d\n",
@@ -1536,7 +1538,7 @@ void vhost_disable_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
15361538
if (vq->used_flags & VRING_USED_F_NO_NOTIFY)
15371539
return;
15381540
vq->used_flags |= VRING_USED_F_NO_NOTIFY;
1539-
if (!vhost_has_feature(dev, VIRTIO_RING_F_EVENT_IDX)) {
1541+
if (!vhost_has_feature(vq, VIRTIO_RING_F_EVENT_IDX)) {
15401542
r = vhost_update_used_flags(vq);
15411543
if (r)
15421544
vq_err(vq, "Failed to enable notification at %p: %d\n",

‎drivers/vhost/vhost.h

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,7 @@ struct vhost_virtqueue {
105105
struct vring_used_elem *heads;
106106
/* Protected by virtqueue mutex. */
107107
void *private_data;
108+
unsigned acked_features;
108109
/* Log write descriptors */
109110
void __user *log_base;
110111
struct vhost_log *log;
@@ -117,7 +118,6 @@ struct vhost_dev {
117118
struct vhost_memory __rcu *memory;
118119
struct mm_struct *mm;
119120
struct mutex mutex;
120-
unsigned acked_features;
121121
struct vhost_virtqueue **vqs;
122122
int nvqs;
123123
struct file *log_file;
@@ -174,13 +174,8 @@ enum {
174174
(1ULL << VHOST_F_LOG_ALL),
175175
};
176176

177-
static inline int vhost_has_feature(struct vhost_dev *dev, int bit)
177+
static inline int vhost_has_feature(struct vhost_virtqueue *vq, int bit)
178178
{
179-
unsigned acked_features;
180-
181-
/* TODO: check that we are running from vhost_worker or dev mutex is
182-
* held? */
183-
acked_features = rcu_dereference_index_check(dev->acked_features, 1);
184-
return acked_features & (1 << bit);
179+
return vq->acked_features & (1 << bit);
185180
}
186181
#endif

0 commit comments

Comments
 (0)