Commit 7b3384fc authored by Michael S. Tsirkin's avatar Michael S. Tsirkin
Browse files

vhost: add unlikely annotations to error path 



patch 'break out of polling loop on error' caused
a minor performance regression on my machine: recover
that performance by adding a bunch of unlikely annotations
in the error handling.

Signed-off-by: default avatarMichael S. Tsirkin <mst@redhat.com>
parent d5675bd2

drivers/vhost/net.c

+2 −2
Original line number Original line Diff line number Diff line
@@ -137,7 +137,7 @@ static void handle_tx(struct vhost_net *net) 
					 &out, &in,

					 &out, &in,

					 NULL, NULL);

					 NULL, NULL);

		/* On error, stop handling until the next kick. */

		/* On error, stop handling until the next kick. */

		if (head < 0)

		if (unlikely(head < 0))

			break;

			break;

		/* Nothing new?  Wait for eventfd to tell us they refilled. */

		/* Nothing new?  Wait for eventfd to tell us they refilled. */

		if (head == vq->num) {

		if (head == vq->num) {
@@ -234,7 +234,7 @@ static void handle_rx(struct vhost_net *net) 
					 &out, &in,

					 &out, &in,

					 vq_log, &log);

					 vq_log, &log);

		/* On error, stop handling until the next kick. */

		/* On error, stop handling until the next kick. */

		if (head < 0)

		if (unlikely(head < 0))

			break;

			break;

		/* OK, now we need to know about added descriptors. */

		/* OK, now we need to know about added descriptors. */

		if (head == vq->num) {

		if (head == vq->num) {

drivers/vhost/vhost.c

+27 −26
Original line number Original line Diff line number Diff line
@@ -736,12 +736,12 @@ static int translate_desc(struct vhost_dev *dev, u64 addr, u32 len, 
	mem = rcu_dereference(dev->memory);

	mem = rcu_dereference(dev->memory);

	while ((u64)len > s) {

	while ((u64)len > s) {

		u64 size;

		u64 size;

		if (ret >= iov_size) {

		if (unlikely(ret >= iov_size)) {

			ret = -ENOBUFS;

			ret = -ENOBUFS;

			break;

			break;

		}

		}

		reg = find_region(mem, addr, len);

		reg = find_region(mem, addr, len);

		if (!reg) {

		if (unlikely(!reg)) {

			ret = -EFAULT;

			ret = -EFAULT;

			break;

			break;

		}

		}
@@ -780,7 +780,7 @@ static unsigned next_desc(struct vring_desc *desc) 
	return next;

	return next;

}

}





static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq,

static int get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq,

			struct iovec iov[], unsigned int iov_size,

			struct iovec iov[], unsigned int iov_size,

			unsigned int *out_num, unsigned int *in_num,

			unsigned int *out_num, unsigned int *in_num,

			struct vhost_log *log, unsigned int *log_num,

			struct vhost_log *log, unsigned int *log_num,
@@ -791,7 +791,7 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
	int ret;

	int ret;





	/* Sanity check */

	/* Sanity check */

	if (indirect->len % sizeof desc) {

	if (unlikely(indirect->len % sizeof desc)) {

		vq_err(vq, "Invalid length in indirect descriptor: "

		vq_err(vq, "Invalid length in indirect descriptor: "

		       "len 0x%llx not multiple of 0x%zx\n",

		       "len 0x%llx not multiple of 0x%zx\n",

		       (unsigned long long)indirect->len,

		       (unsigned long long)indirect->len,
@@ -801,7 +801,7 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




	ret = translate_desc(dev, indirect->addr, indirect->len, vq->indirect,

	ret = translate_desc(dev, indirect->addr, indirect->len, vq->indirect,

			     ARRAY_SIZE(vq->indirect));

			     ARRAY_SIZE(vq->indirect));

	if (ret < 0) {

	if (unlikely(ret < 0)) {

		vq_err(vq, "Translation failure %d in indirect.\n", ret);

		vq_err(vq, "Translation failure %d in indirect.\n", ret);

		return ret;

		return ret;

	}

	}
@@ -813,7 +813,7 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
	count = indirect->len / sizeof desc;

	count = indirect->len / sizeof desc;

	/* Buffers are chained via a 16 bit next field, so

	/* Buffers are chained via a 16 bit next field, so

	 * we can have at most 2^16 of these. */

	 * we can have at most 2^16 of these. */

	if (count > USHRT_MAX + 1) {

	if (unlikely(count > USHRT_MAX + 1)) {

		vq_err(vq, "Indirect buffer length too big: %d\n",

		vq_err(vq, "Indirect buffer length too big: %d\n",

		       indirect->len);

		       indirect->len);

		return -E2BIG;

		return -E2BIG;
@@ -821,19 +821,19 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




	do {

	do {

		unsigned iov_count = *in_num + *out_num;

		unsigned iov_count = *in_num + *out_num;

		if (++found > count) {

		if (unlikely(++found > count)) {

			vq_err(vq, "Loop detected: last one at %u "

			vq_err(vq, "Loop detected: last one at %u "

			       "indirect size %u\n",

			       "indirect size %u\n",

			       i, count);

			       i, count);

			return -EINVAL;

			return -EINVAL;

		}

		}

		if (memcpy_fromiovec((unsigned char *)&desc, vq->indirect,

		if (unlikely(memcpy_fromiovec((unsigned char *)&desc, vq->indirect,

				     sizeof desc)) {

					      sizeof desc))) {

			vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",

			vq_err(vq, "Failed indirect descriptor: idx %d, %zx\n",

			       i, (size_t)indirect->addr + i * sizeof desc);

			       i, (size_t)indirect->addr + i * sizeof desc);

			return -EINVAL;

			return -EINVAL;

		}

		}

		if (desc.flags & VRING_DESC_F_INDIRECT) {

		if (unlikely(desc.flags & VRING_DESC_F_INDIRECT)) {

			vq_err(vq, "Nested indirect descriptor: idx %d, %zx\n",

			vq_err(vq, "Nested indirect descriptor: idx %d, %zx\n",

			       i, (size_t)indirect->addr + i * sizeof desc);

			       i, (size_t)indirect->addr + i * sizeof desc);

			return -EINVAL;

			return -EINVAL;
@@ -841,7 +841,7 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




		ret = translate_desc(dev, desc.addr, desc.len, iov + iov_count,

		ret = translate_desc(dev, desc.addr, desc.len, iov + iov_count,

				     iov_size - iov_count);

				     iov_size - iov_count);

		if (ret < 0) {

		if (unlikely(ret < 0)) {

			vq_err(vq, "Translation failure %d indirect idx %d\n",

			vq_err(vq, "Translation failure %d indirect idx %d\n",

			       ret, i);

			       ret, i);

			return ret;

			return ret;
@@ -857,7 +857,7 @@ static unsigned get_indirect(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
		} else {

		} else {

			/* If it's an output descriptor, they're all supposed

			/* If it's an output descriptor, they're all supposed

			 * to come before any input descriptors. */

			 * to come before any input descriptors. */

			if (*in_num) {

			if (unlikely(*in_num)) {

				vq_err(vq, "Indirect descriptor "

				vq_err(vq, "Indirect descriptor "

				       "has out after in: idx %d\n", i);

				       "has out after in: idx %d\n", i);

				return -EINVAL;

				return -EINVAL;
@@ -888,13 +888,13 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




	/* Check it isn't doing very strange things with descriptor numbers. */

	/* Check it isn't doing very strange things with descriptor numbers. */

	last_avail_idx = vq->last_avail_idx;

	last_avail_idx = vq->last_avail_idx;

	if (get_user(vq->avail_idx, &vq->avail->idx)) {

	if (unlikely(get_user(vq->avail_idx, &vq->avail->idx))) {

		vq_err(vq, "Failed to access avail idx at %p\n",

		vq_err(vq, "Failed to access avail idx at %p\n",

		       &vq->avail->idx);

		       &vq->avail->idx);

		return -EFAULT;

		return -EFAULT;

	}

	}





	if ((u16)(vq->avail_idx - last_avail_idx) > vq->num) {

	if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {

		vq_err(vq, "Guest moved used index from %u to %u",

		vq_err(vq, "Guest moved used index from %u to %u",

		       last_avail_idx, vq->avail_idx);

		       last_avail_idx, vq->avail_idx);

		return -EFAULT;

		return -EFAULT;
@@ -909,7 +909,8 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




	/* Grab the next descriptor number they're advertising, and increment

	/* Grab the next descriptor number they're advertising, and increment

	 * the index we've seen. */

	 * the index we've seen. */

	if (get_user(head, &vq->avail->ring[last_avail_idx % vq->num])) {

	if (unlikely(get_user(head,

			      &vq->avail->ring[last_avail_idx % vq->num]))) {

		vq_err(vq, "Failed to read head: idx %d address %p\n",

		vq_err(vq, "Failed to read head: idx %d address %p\n",

		       last_avail_idx,

		       last_avail_idx,

		       &vq->avail->ring[last_avail_idx % vq->num]);

		       &vq->avail->ring[last_avail_idx % vq->num]);
@@ -917,7 +918,7 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
	}

	}





	/* If their number is silly, that's an error. */

	/* If their number is silly, that's an error. */

	if (head >= vq->num) {

	if (unlikely(head >= vq->num)) {

		vq_err(vq, "Guest says index %u > %u is available",

		vq_err(vq, "Guest says index %u > %u is available",

		       head, vq->num);

		       head, vq->num);

		return -EINVAL;

		return -EINVAL;
@@ -931,19 +932,19 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
	i = head;

	i = head;

	do {

	do {

		unsigned iov_count = *in_num + *out_num;

		unsigned iov_count = *in_num + *out_num;

		if (i >= vq->num) {

		if (unlikely(i >= vq->num)) {

			vq_err(vq, "Desc index is %u > %u, head = %u",

			vq_err(vq, "Desc index is %u > %u, head = %u",

			       i, vq->num, head);

			       i, vq->num, head);

			return -EINVAL;

			return -EINVAL;

		}

		}

		if (++found > vq->num) {

		if (unlikely(++found > vq->num)) {

			vq_err(vq, "Loop detected: last one at %u "

			vq_err(vq, "Loop detected: last one at %u "

			       "vq size %u head %u\n",

			       "vq size %u head %u\n",

			       i, vq->num, head);

			       i, vq->num, head);

			return -EINVAL;

			return -EINVAL;

		}

		}

		ret = copy_from_user(&desc, vq->desc + i, sizeof desc);

		ret = copy_from_user(&desc, vq->desc + i, sizeof desc);

		if (ret) {

		if (unlikely(ret)) {

			vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",

			vq_err(vq, "Failed to get descriptor: idx %d addr %p\n",

			       i, vq->desc + i);

			       i, vq->desc + i);

			return -EFAULT;

			return -EFAULT;
@@ -952,7 +953,7 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
			ret = get_indirect(dev, vq, iov, iov_size,

			ret = get_indirect(dev, vq, iov, iov_size,

					   out_num, in_num,

					   out_num, in_num,

					   log, log_num, &desc);

					   log, log_num, &desc);

			if (ret < 0) {

			if (unlikely(ret < 0)) {

				vq_err(vq, "Failure detected "

				vq_err(vq, "Failure detected "

				       "in indirect descriptor at idx %d\n", i);

				       "in indirect descriptor at idx %d\n", i);

				return ret;

				return ret;
@@ -962,7 +963,7 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 




		ret = translate_desc(dev, desc.addr, desc.len, iov + iov_count,

		ret = translate_desc(dev, desc.addr, desc.len, iov + iov_count,

				     iov_size - iov_count);

				     iov_size - iov_count);

		if (ret < 0) {

		if (unlikely(ret < 0)) {

			vq_err(vq, "Translation failure %d descriptor idx %d\n",

			vq_err(vq, "Translation failure %d descriptor idx %d\n",

			       ret, i);

			       ret, i);

			return ret;

			return ret;
@@ -979,7 +980,7 @@ int vhost_get_vq_desc(struct vhost_dev *dev, struct vhost_virtqueue *vq, 
		} else {

		} else {

			/* If it's an output descriptor, they're all supposed

			/* If it's an output descriptor, they're all supposed

			 * to come before any input descriptors. */

			 * to come before any input descriptors. */

			if (*in_num) {

			if (unlikely(*in_num)) {

				vq_err(vq, "Descriptor has out after in: "

				vq_err(vq, "Descriptor has out after in: "

				       "idx %d\n", i);

				       "idx %d\n", i);

				return -EINVAL;

				return -EINVAL;