ivampiresp/Spdk
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

vhost: fetch and check avail_idx from memory

Browse Source 
Since there may be race condition, num_events gotten
from kickfd is not always right to reflect the number
of requests in virtqueue. So remove this check.
Instead, avail_idx in memory reflects the real situation
whether there is unprocessed requests in queue.

Change-Id: I4e59242cf3101ece912777533411bc4dcdc81c9d
Signed-off-by: Liu Xiaodong <xiaodong.liu@intel.com>
Reviewed-on: https://review.spdk.io/gerrit/c/spdk/spdk/+/5787
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Community-CI: Mellanox Build Bot
Reviewed-by: Changpeng Liu <changpeng.liu@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>
This commit is contained in:
Liu Xiaodong 2021-01-06 03:26:18 -05:00 committed by Jim Harris
parent fce2956c24
commit 7a3246bfb2
1 changed files with 13 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
lib/vhost/vhost.c
View File

@ -184,23 +184,17 @@ vhost_vq_avail_ring_get(struct spdk_vhost_virtqueue *virtqueue, uint16_t *reqs,
uint16_t last_idx = virtqueue->last_avail_idx, avail_idx = avail->idx; uint16_t last_idx = virtqueue->last_avail_idx, avail_idx = avail->idx;
uint16_t count, i; uint16_t count, i;
int rc; int rc;
uint64_t num_events; uint64_t u64_value;
spdk_smp_rmb(); spdk_smp_rmb();
if (virtqueue->vsession && spdk_unlikely(virtqueue->vsession->interrupt_mode)) { if (virtqueue->vsession && spdk_unlikely(virtqueue->vsession->interrupt_mode)) {
/* Acknowledge vring's kickfd */ /* Read to clear vring's kickfd */
rc = read(vring->kickfd, &num_events, sizeof(num_events)); rc = read(vring->kickfd, &u64_value, sizeof(u64_value));
if (rc < 0) { if (rc < 0) {
SPDK_ERRLOG("failed to acknowledge kickfd: %s.\n", spdk_strerror(errno)); SPDK_ERRLOG("failed to acknowledge kickfd: %s.\n", spdk_strerror(errno));
return -errno; return -errno;
} }
if ((uint16_t)(avail_idx - last_idx) != num_events) {
SPDK_DEBUGLOG(vhost_ring,
"virtqueue gets %d reqs, but kickfd shows %lu reqs\n",
avail_idx - last_idx, num_events);
}
} }
count = avail_idx - last_idx; count = avail_idx - last_idx;
@ -216,13 +210,17 @@ vhost_vq_avail_ring_get(struct spdk_vhost_virtqueue *virtqueue, uint16_t *reqs,
} }
count = spdk_min(count, reqs_len); count = spdk_min(count, reqs_len);
virtqueue->last_avail_idx += count;
/* Check whether there are unprocessed reqs in vq, then kick vq manually */
if (virtqueue->vsession && spdk_unlikely(virtqueue->vsession->interrupt_mode)) { if (virtqueue->vsession && spdk_unlikely(virtqueue->vsession->interrupt_mode)) {
if (num_events > count) { /* If avail_idx is larger than virtqueue's last_avail_idx, then there is unprocessed reqs.
SPDK_DEBUGLOG(vhost_ring, * avail_idx should get updated here from memory, in case of race condition with guest.
"virtqueue kickfd shows %lu reqs, take %d, send notice for other reqs\n", */
num_events, reqs_len); avail_idx = * (volatile uint16_t *) &avail->idx;
num_events -= count; if (avail_idx > virtqueue->last_avail_idx) {
rc = write(vring->kickfd, &num_events, sizeof(num_events)); /* Write to notify vring's kickfd */
rc = write(vring->kickfd, &u64_value, sizeof(u64_value));
if (rc < 0) { if (rc < 0) {
SPDK_ERRLOG("failed to kick vring: %s.\n", spdk_strerror(errno)); SPDK_ERRLOG("failed to kick vring: %s.\n", spdk_strerror(errno));
return -errno; return -errno;
@ -230,7 +228,6 @@ vhost_vq_avail_ring_get(struct spdk_vhost_virtqueue *virtqueue, uint16_t *reqs,
} }
} }
virtqueue->last_avail_idx += count;
for (i = 0; i < count; i++) { for (i = 0; i < count; i++) {
reqs[i] = vring->avail->ring[(last_idx + i) & size_mask]; reqs[i] = vring->avail->ring[(last_idx + i) & size_mask];
} }