Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH] vhost: Coalesce vq_err formats, pr_fmt misuse, add missing newlines
From: Joe Perches @ 2017-05-22 10:33 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization

vhost logging uses of vq_err has a few defects and style inconsistencies.

o pr_debug already uses pr_fmt so its use in vq_err is defective
  however no #defines of pr_fmt exist in this code so no actual
  defects exist
o vq_err uses need terminating newlines so add the missing ones
o Coalesce formats and realign arguments

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/vhost/net.c   | 17 ++++++-------
 drivers/vhost/scsi.c  | 17 ++++++-------
 drivers/vhost/test.c  |  4 +--
 drivers/vhost/vhost.c | 70 +++++++++++++++++++++++----------------------------
 drivers/vhost/vhost.h | 11 ++++----
 5 files changed, 54 insertions(+), 65 deletions(-)

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index e3d7ea1288c6..7c8c013381e1 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -501,8 +501,8 @@ static void handle_tx(struct vhost_net *net)
 			break;
 		}
 		if (in) {
-			vq_err(vq, "Unexpected descriptor format for TX: "
-			       "out %d, int %d\n", out, in);
+			vq_err(vq, "Unexpected descriptor format for TX: out %d, int %d\n",
+			       out, in);
 			break;
 		}
 		/* Skip header. TODO: support TSO. */
@@ -511,8 +511,7 @@ static void handle_tx(struct vhost_net *net)
 		iov_iter_advance(&msg.msg_iter, hdr_size);
 		/* Sanity check */
 		if (!msg_data_left(&msg)) {
-			vq_err(vq, "Unexpected header len for TX: "
-			       "%zd expected %zd\n",
+			vq_err(vq, "Unexpected header len for TX: %zd expected %zd\n",
 			       len, hdr_size);
 			break;
 		}
@@ -689,8 +688,8 @@ static int get_rx_bufs(struct vhost_virtqueue *vq,
 			goto err;
 		}
 		if (unlikely(out || in <= 0)) {
-			vq_err(vq, "unexpected descriptor format for RX: "
-				"out %d, in %d\n", out, in);
+			vq_err(vq, "unexpected descriptor format for RX: out %d, in %d\n",
+			       out, in);
 			r = -EINVAL;
 			goto err;
 		}
@@ -822,8 +821,8 @@ static void handle_rx(struct vhost_net *net)
 		if (unlikely(vhost_hlen)) {
 			if (copy_to_iter(&hdr, sizeof(hdr),
 					 &fixup) != sizeof(hdr)) {
-				vq_err(vq, "Unable to write vnet_hdr "
-				       "at addr %p\n", vq->iov->iov_base);
+				vq_err(vq, "Unable to write vnet_hdr at addr %p\n",
+				       vq->iov->iov_base);
 				goto out;
 			}
 		} else {
@@ -838,7 +837,7 @@ static void handle_rx(struct vhost_net *net)
 		if (likely(mergeable) &&
 		    copy_to_iter(&num_buffers, sizeof num_buffers,
 				 &fixup) != sizeof num_buffers) {
-			vq_err(vq, "Failed num_buffers write");
+			vq_err(vq, "Failed num_buffers write\n");
 			vhost_discard_vq_desc(vq, headcount);
 			goto out;
 		}
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index fd6c8b66f06f..c0d3746d5ff3 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -473,7 +473,7 @@ vhost_scsi_do_evt_work(struct vhost_scsi *vs, struct vhost_scsi_evt *evt)
 
 	if ((vq->iov[out].iov_len != sizeof(struct virtio_scsi_event))) {
 		vq_err(vq, "Expecting virtio_scsi_event, got %zu bytes\n",
-				vq->iov[out].iov_len);
+		       vq->iov[out].iov_len);
 		vs->vs_events_missed = true;
 		return;
 	}
@@ -885,8 +885,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 		 * errors back to the guest.
 		 */
 		if (unlikely(vq->iov[out].iov_len < rsp_size)) {
-			vq_err(vq, "Expecting at least virtio_scsi_cmd_resp"
-				" size, got %zu bytes\n", vq->iov[out].iov_len);
+			vq_err(vq, "Expecting at least virtio_scsi_cmd_resp size, got %zu bytes\n",
+			       vq->iov[out].iov_len);
 			break;
 		}
 		/*
@@ -981,16 +981,14 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 		if (t10_pi) {
 			if (v_req_pi.pi_bytesout) {
 				if (data_direction != DMA_TO_DEVICE) {
-					vq_err(vq, "Received non zero pi_bytesout,"
-						" but wrong data_direction\n");
+					vq_err(vq, "Received non zero pi_bytesout, but wrong data_direction\n");
 					vhost_scsi_send_bad_target(vs, vq, head, out);
 					continue;
 				}
 				prot_bytes = vhost32_to_cpu(vq, v_req_pi.pi_bytesout);
 			} else if (v_req_pi.pi_bytesin) {
 				if (data_direction != DMA_FROM_DEVICE) {
-					vq_err(vq, "Received non zero pi_bytesin,"
-						" but wrong data_direction\n");
+					vq_err(vq, "Received non zero pi_bytesin, but wrong data_direction\n");
 					vhost_scsi_send_bad_target(vs, vq, head, out);
 					continue;
 				}
@@ -1026,9 +1024,8 @@ vhost_scsi_handle_vq(struct vhost_scsi *vs, struct vhost_virtqueue *vq)
 		 * TODO what if cdb was too small for varlen cdb header?
 		 */
 		if (unlikely(scsi_command_size(cdb) > VHOST_SCSI_MAX_CDB_SIZE)) {
-			vq_err(vq, "Received SCSI CDB with command_size: %d that"
-				" exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
-				scsi_command_size(cdb), VHOST_SCSI_MAX_CDB_SIZE);
+			vq_err(vq, "Received SCSI CDB with command_size: %d that exceeds SCSI_MAX_VARLEN_CDB_SIZE: %d\n",
+			       scsi_command_size(cdb), VHOST_SCSI_MAX_CDB_SIZE);
 			vhost_scsi_send_bad_target(vs, vq, head, out);
 			continue;
 		}
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index 3cc98c07dcd3..a48e9747505d 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -69,8 +69,8 @@ static void handle_vq(struct vhost_test *n)
 			break;
 		}
 		if (in) {
-			vq_err(vq, "Unexpected descriptor format for TX: "
-			       "out %d, int %d\n", out, in);
+			vq_err(vq, "Unexpected descriptor format for TX: out %d, int %d\n",
+			       out, in);
 			break;
 		}
 		len = iov_length(vq->iov, out);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 042030e5a035..6730735d31c7 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -802,9 +802,8 @@ static int vhost_copy_from_user(struct vhost_virtqueue *vq, void *to,
 				     ARRAY_SIZE(vq->iotlb_iov),
 				     VHOST_ACCESS_RO);
 		if (ret < 0) {
-			vq_err(vq, "IOTLB translation failure: uaddr "
-			       "%p size 0x%llx\n", from,
-			       (unsigned long long) size);
+			vq_err(vq, "IOTLB translation failure: uaddr %p size 0x%llx\n",
+			       from, (unsigned long long)size);
 			goto out;
 		}
 		iov_iter_init(&f, READ, vq->iotlb_iov, ret, size);
@@ -827,16 +826,14 @@ static void __user *__vhost_get_user_slow(struct vhost_virtqueue *vq,
 			     ARRAY_SIZE(vq->iotlb_iov),
 			     VHOST_ACCESS_RO);
 	if (ret < 0) {
-		vq_err(vq, "IOTLB translation failure: uaddr "
-			"%p size 0x%llx\n", addr,
-			(unsigned long long) size);
+		vq_err(vq, "IOTLB translation failure: uaddr %p size 0x%llx\n",
+		       addr, (unsigned long long)size);
 		return NULL;
 	}
 
 	if (ret != 1 || vq->iotlb_iov[0].iov_len != size) {
-		vq_err(vq, "Non atomic userspace memory access: uaddr "
-			"%p size 0x%llx\n", addr,
-			(unsigned long long) size);
+		vq_err(vq, "Non atomic userspace memory access: uaddr %p size 0x%llx\n",
+		       addr, (unsigned long long)size);
 		return NULL;
 	}
 
@@ -1807,8 +1804,7 @@ int vhost_vq_init_access(struct vhost_virtqueue *vq)
 	}
 	r = vhost_get_used(vq, last_used_idx, &vq->used->idx);
 	if (r) {
-		vq_err(vq, "Can't access used idx at %p\n",
-		       &vq->used->idx);
+		vq_err(vq, "Can't access used idx at %p\n", &vq->used->idx);
 		goto err;
 	}
 	vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx);
@@ -1901,10 +1897,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
 
 	/* Sanity check */
 	if (unlikely(len % sizeof desc)) {
-		vq_err(vq, "Invalid length in indirect descriptor: "
-		       "len 0x%llx not multiple of 0x%zx\n",
-		       (unsigned long long)len,
-		       sizeof desc);
+		vq_err(vq, "Invalid length in indirect descriptor: len 0x%llx not multiple of 0x%zx\n",
+		       (unsigned long long)len, sizeof(desc));
 		return -EINVAL;
 	}
 
@@ -1912,7 +1906,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
 			     UIO_MAXIOV, VHOST_ACCESS_RO);
 	if (unlikely(ret < 0)) {
 		if (ret != -EAGAIN)
-			vq_err(vq, "Translation failure %d in indirect.\n", ret);
+			vq_err(vq, "Translation failure %d in indirect\n", ret);
 		return ret;
 	}
 	iov_iter_init(&from, READ, vq->indirect, ret, len);
@@ -1933,8 +1927,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
 	do {
 		unsigned iov_count = *in_num + *out_num;
 		if (unlikely(++found > count)) {
-			vq_err(vq, "Loop detected: last one at %u "
-			       "indirect size %u\n",
+			vq_err(vq, "Loop detected: last one at %u indirect size %u\n",
 			       i, count);
 			return -EINVAL;
 		}
@@ -1960,7 +1953,7 @@ static int get_indirect(struct vhost_virtqueue *vq,
 		if (unlikely(ret < 0)) {
 			if (ret != -EAGAIN)
 				vq_err(vq, "Translation failure %d indirect idx %d\n",
-					ret, i);
+				       ret, i);
 			return ret;
 		}
 		/* If this is an input descriptor, increment that count. */
@@ -1975,8 +1968,8 @@ static int get_indirect(struct vhost_virtqueue *vq,
 			/* If it's an output descriptor, they're all supposed
 			 * to come before any input descriptors. */
 			if (unlikely(*in_num)) {
-				vq_err(vq, "Indirect descriptor "
-				       "has out after in: idx %d\n", i);
+				vq_err(vq, "Indirect descriptor has out after in: idx %d\n",
+				       i);
 				return -EINVAL;
 			}
 			*out_num += ret;
@@ -2011,14 +2004,14 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 	if (vq->avail_idx == vq->last_avail_idx) {
 		if (unlikely(vhost_get_avail(vq, avail_idx, &vq->avail->idx))) {
 			vq_err(vq, "Failed to access avail idx at %p\n",
-				&vq->avail->idx);
+			       &vq->avail->idx);
 			return -EFAULT;
 		}
 		vq->avail_idx = vhost16_to_cpu(vq, avail_idx);
 
 		if (unlikely((u16)(vq->avail_idx - last_avail_idx) > vq->num)) {
-			vq_err(vq, "Guest moved used index from %u to %u",
-				last_avail_idx, vq->avail_idx);
+			vq_err(vq, "Guest moved used index from %u to %u\n",
+			       last_avail_idx, vq->avail_idx);
 			return -EFAULT;
 		}
 
@@ -2048,7 +2041,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 
 	/* If their number is silly, that's an error. */
 	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\n",
 		       head, vq->num);
 		return -EINVAL;
 	}
@@ -2062,13 +2055,12 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 	do {
 		unsigned iov_count = *in_num + *out_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\n",
 			       i, vq->num, head);
 			return -EINVAL;
 		}
 		if (unlikely(++found > vq->num)) {
-			vq_err(vq, "Loop detected: last one at %u "
-			       "vq size %u head %u\n",
+			vq_err(vq, "Loop detected: last one at %u vq size %u head %u\n",
 			       i, vq->num, head);
 			return -EINVAL;
 		}
@@ -2085,8 +2077,8 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 					   log, log_num, &desc);
 			if (unlikely(ret < 0)) {
 				if (ret != -EAGAIN)
-					vq_err(vq, "Failure detected "
-						"in indirect descriptor at idx %d\n", i);
+					vq_err(vq, "Failure detected in indirect descriptor at idx %d\n",
+					       i);
 				return ret;
 			}
 			continue;
@@ -2102,7 +2094,7 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 		if (unlikely(ret < 0)) {
 			if (ret != -EAGAIN)
 				vq_err(vq, "Translation failure %d descriptor idx %d\n",
-					ret, i);
+				       ret, i);
 			return ret;
 		}
 		if (access == VHOST_ACCESS_WO) {
@@ -2118,8 +2110,8 @@ int vhost_get_vq_desc(struct vhost_virtqueue *vq,
 			/* If it's an output descriptor, they're all supposed
 			 * to come before any input descriptors. */
 			if (unlikely(*in_num)) {
-				vq_err(vq, "Descriptor has out after in: "
-				       "idx %d\n", i);
+				vq_err(vq, "Descriptor has out after in: idx %d\n",
+				       i);
 				return -EINVAL;
 			}
 			*out_num += ret;
@@ -2168,15 +2160,15 @@ static int __vhost_add_used_n(struct vhost_virtqueue *vq,
 	used = vq->used->ring + start;
 	if (count == 1) {
 		if (vhost_put_user(vq, heads[0].id, &used->id)) {
-			vq_err(vq, "Failed to write used id");
+			vq_err(vq, "Failed to write used id\n");
 			return -EFAULT;
 		}
 		if (vhost_put_user(vq, heads[0].len, &used->len)) {
-			vq_err(vq, "Failed to write used len");
+			vq_err(vq, "Failed to write used len\n");
 			return -EFAULT;
 		}
 	} else if (vhost_copy_to_user(vq, used, heads, count * sizeof *used)) {
-		vq_err(vq, "Failed to write used");
+		vq_err(vq, "Failed to write used\n");
 		return -EFAULT;
 	}
 	if (unlikely(vq->log_used)) {
@@ -2221,7 +2213,7 @@ int vhost_add_used_n(struct vhost_virtqueue *vq, struct vring_used_elem *heads,
 	smp_wmb();
 	if (vhost_put_user(vq, cpu_to_vhost16(vq, vq->last_used_idx),
 			   &vq->used->idx)) {
-		vq_err(vq, "Failed to increment used idx");
+		vq_err(vq, "Failed to increment used idx\n");
 		return -EFAULT;
 	}
 	if (unlikely(vq->log_used)) {
@@ -2253,7 +2245,7 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 		 * interrupts. */
 		smp_mb();
 		if (vhost_get_avail(vq, flags, &vq->avail->flags)) {
-			vq_err(vq, "Failed to get flags");
+			vq_err(vq, "Failed to get flags\n");
 			return true;
 		}
 		return !(flags & cpu_to_vhost16(vq, VRING_AVAIL_F_NO_INTERRUPT));
@@ -2280,7 +2272,7 @@ static bool vhost_notify(struct vhost_dev *dev, struct vhost_virtqueue *vq)
 	smp_mb();
 
 	if (vhost_get_avail(vq, event, vhost_used_event(vq))) {
-		vq_err(vq, "Failed to get used event idx");
+		vq_err(vq, "Failed to get used event idx\n");
 		return true;
 	}
 	vq->last_used_event = vhost16_to_cpu(vq, event);
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index f55671d53f28..18bcfc70459a 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -227,11 +227,12 @@ ssize_t vhost_chr_write_iter(struct vhost_dev *dev,
 			     struct iov_iter *from);
 int vhost_init_device_iotlb(struct vhost_dev *d, bool enabled);
 
-#define vq_err(vq, fmt, ...) do {                                  \
-		pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \
-		if ((vq)->error_ctx)                               \
-				eventfd_signal((vq)->error_ctx, 1);\
-	} while (0)
+#define vq_err(vq, fmt, ...)						\
+do {									\
+	pr_debug(fmt, ##__VA_ARGS__);					\
+	if ((vq)->error_ctx)						\
+		eventfd_signal((vq)->error_ctx, 1);			\
+} while (0)
 
 enum {
 	VHOST_FEATURES = (1ULL << VIRTIO_F_NOTIFY_ON_EMPTY) |
-- 
2.10.0.rc2.1.g053435c

^ permalink raw reply related

* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
From: SF Markus Elfring @ 2017-05-22 10:50 UTC (permalink / raw)
  To: Stefan Hajnoczi, kvm, netdev, virtualization, kernel-janitors
  Cc: Wolfram Sang, LKML, Michael S. Tsirkin
In-Reply-To: <20170522094320.GE12205@stefanha-x1.localdomain>

>> Link: http://events.linuxfoundation.org/sites/events/files/slides/LCJ16-Refactor_Strings-WSang_0.pdf
> 
> Please include an actual explanation for this change instead of linking
> to slides.

Do you care for a bit of code size reduction by removal of questionable
error messages?


> Why are you trying to get rid of memory allocation failure messages?

Do you find information from a Linux allocation failure report sufficient
for any function implementations here?


>> +++ b/drivers/vhost/scsi.c
>> @@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
>>  	if (!evt) {
>> -		vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
> 
> #define vq_err(vq, fmt, ...) do {                                  \
>                 pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \
>                 if ((vq)->error_ctx)                               \
>                                 eventfd_signal((vq)->error_ctx, 1);\
>         } while (0)
> 
> You silently dropped the eventfd_signal() call.

Do you prefer to preserve this special error handling then?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
From: Stefan Hajnoczi @ 2017-05-22 11:23 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: kvm, Michael S. Tsirkin, netdev, Wolfram Sang, kernel-janitors,
	LKML, virtualization
In-Reply-To: <557bdff4-6dc2-756d-0d59-9f688ef11f0a@users.sourceforge.net>


[-- Attachment #1.1: Type: text/plain, Size: 1309 bytes --]

On Mon, May 22, 2017 at 12:50:39PM +0200, SF Markus Elfring wrote:
> > Why are you trying to get rid of memory allocation failure messages?
> 
> Do you find information from a Linux allocation failure report sufficient
> for any function implementations here?

If kmalloc() and friends guarantee to print a warning and backtrace on
every allocation failure, then there's no need for error messages in
callers.

That seems like good justification that can go in the commit
description, but I'm not sure if kmalloc() and friends guarantee to show
a message (not just the first time, but for every failed allocation)?

> >> +++ b/drivers/vhost/scsi.c
> >> @@ -417,5 +417,4 @@ vhost_scsi_allocate_evt(struct vhost_scsi *vs,
> >>  	if (!evt) {
> >> -		vq_err(vq, "Failed to allocate vhost_scsi_evt\n");
> > 
> > #define vq_err(vq, fmt, ...) do {                                  \
> >                 pr_debug(pr_fmt(fmt), ##__VA_ARGS__);       \
> >                 if ((vq)->error_ctx)                               \
> >                                 eventfd_signal((vq)->error_ctx, 1);\
> >         } while (0)
> > 
> > You silently dropped the eventfd_signal() call.
> 
> Do you prefer to preserve this special error handling then?

Yes, please leave vq_err() calls.

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
From: SF Markus Elfring @ 2017-05-22 11:34 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: kvm, Michael S. Tsirkin, netdev, Wolfram Sang, kernel-janitors,
	LKML, virtualization
In-Reply-To: <20170522112320.GA22806@stefanha-x1.localdomain>

>> Do you find information from a Linux allocation failure report sufficient
>> for any function implementations here?
> 
> If kmalloc() and friends guarantee to print a warning and backtrace on
> every allocation failure, then there's no need for error messages in
> callers.
> 
> That seems like good justification that can go in the commit
> description, but I'm not sure if kmalloc() and friends guarantee to show
> a message (not just the first time, but for every failed allocation)?

I am also looking for a more complete and easier accessible documentation
for this aspect of the desired exception handling.
How would we like to resolve any remaining open issues there?

Regards,
Markus

^ permalink raw reply

* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
From: Dan Carpenter @ 2017-05-22 12:38 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: kvm, Michael S. Tsirkin, netdev, Wolfram Sang, kernel-janitors,
	LKML, virtualization, SF Markus Elfring
In-Reply-To: <20170522112320.GA22806@stefanha-x1.localdomain>

On Mon, May 22, 2017 at 12:23:20PM +0100, Stefan Hajnoczi wrote:
> I'm not sure if kmalloc() and friends guarantee to show
> a message (not just the first time, but for every failed allocation)?
>

It prints multiple times, but it's ratelimited.  It can also be disabled
using a config option.

See slab_out_of_memory().

regards,
dan carpenter

^ permalink raw reply

* Re: [RFC PATCH kvmtool 00/15] Add virtio-iommu
From: Jean-Philippe Brucker @ 2017-05-22 14:01 UTC (permalink / raw)
  To: Bharat Bhushan, iommu@lists.linux-foundation.org,
	kvm@vger.kernel.org, virtualization@lists.linux-foundation.org,
	virtio-dev@lists.oasis-open.org
  Cc: cdall@linaro.org, lorenzo.pieralisi@arm.com, mst@redhat.com,
	marc.zyngier@arm.com, joro@8bytes.org, will.deacon@arm.com,
	robin.murphy@arm.com
In-Reply-To: <AM5PR0401MB2545FADDF2A7649DF0DB68309AF80@AM5PR0401MB2545.eurprd04.prod.outlook.com>

Hi Bharat,

On 22/05/17 09:26, Bharat Bhushan wrote:
> Hi Jean,
> 
> I am trying to run and review on my side but I see Linux patches are not with latest kernel version.
> Will it be possible for you to share your Linux and kvmtool git repository reference?

Please find linux and kvmtool patches at the following repos:

git://linux-arm.org/kvmtool-jpb.git virtio-iommu/base
git://linux-arm.org/linux-jpb.git virtio-iommu/base

Note that these branches are unstable, subject to fixes and rebase. I'll
try to keep them in sync with upstream.

Thanks,
Jean

^ permalink raw reply

* Re: [PATCH 2/2] vhost/scsi: Delete error messages for failed memory allocations in five functions
From: Stefan Hajnoczi @ 2017-05-22 14:08 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: kvm, Michael S. Tsirkin, netdev, Wolfram Sang, kernel-janitors,
	LKML, virtualization, SF Markus Elfring
In-Reply-To: <20170522123833.asx2biaw4xaz4qpv@mwanda>


[-- Attachment #1.1: Type: text/plain, Size: 433 bytes --]

On Mon, May 22, 2017 at 03:38:33PM +0300, Dan Carpenter wrote:
> On Mon, May 22, 2017 at 12:23:20PM +0100, Stefan Hajnoczi wrote:
> > I'm not sure if kmalloc() and friends guarantee to show
> > a message (not just the first time, but for every failed allocation)?
> >
> 
> It prints multiple times, but it's ratelimited.  It can also be disabled
> using a config option.
> 
> See slab_out_of_memory().

Thanks!

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
From: Stefan Hajnoczi @ 2017-05-22 14:09 UTC (permalink / raw)
  To: SF Markus Elfring
  Cc: kvm, Michael S. Tsirkin, netdev, Wolfram Sang, kernel-janitors,
	LKML, virtualization
In-Reply-To: <e6658e56-a2b0-4ecc-0a95-0ba193ff2c97@users.sourceforge.net>


[-- Attachment #1.1: Type: text/plain, Size: 861 bytes --]

On Mon, May 22, 2017 at 01:34:34PM +0200, SF Markus Elfring wrote:
> >> Do you find information from a Linux allocation failure report sufficient
> >> for any function implementations here?
> > 
> > If kmalloc() and friends guarantee to print a warning and backtrace on
> > every allocation failure, then there's no need for error messages in
> > callers.
> > 
> > That seems like good justification that can go in the commit
> > description, but I'm not sure if kmalloc() and friends guarantee to show
> > a message (not just the first time, but for every failed allocation)?
> 
> I am also looking for a more complete and easier accessible documentation
> for this aspect of the desired exception handling.
> How would we like to resolve any remaining open issues there?

No objection from me but please make sure to keep vq_err().

Stefan

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH] vhost: Coalesce vq_err formats, pr_fmt misuse, add missing newlines
From: Stefan Hajnoczi @ 2017-05-22 14:16 UTC (permalink / raw)
  To: Joe Perches; +Cc: kvm, Michael S. Tsirkin, netdev, linux-kernel, virtualization
In-Reply-To: <0e8f84aec009831d34ccb2b4ed27364e78b07e02.1495449147.git.joe@perches.com>


[-- Attachment #1.1: Type: text/plain, Size: 824 bytes --]

On Mon, May 22, 2017 at 03:33:43AM -0700, Joe Perches wrote:
> vhost logging uses of vq_err has a few defects and style inconsistencies.
> 
> o pr_debug already uses pr_fmt so its use in vq_err is defective
>   however no #defines of pr_fmt exist in this code so no actual
>   defects exist
> o vq_err uses need terminating newlines so add the missing ones
> o Coalesce formats and realign arguments
> 
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  drivers/vhost/net.c   | 17 ++++++-------
>  drivers/vhost/scsi.c  | 17 ++++++-------
>  drivers/vhost/test.c  |  4 +--
>  drivers/vhost/vhost.c | 70 +++++++++++++++++++++++----------------------------
>  drivers/vhost/vhost.h | 11 ++++----
>  5 files changed, 54 insertions(+), 65 deletions(-)

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: vhost/scsi: Delete error messages for failed memory allocations in five functions
From: SF Markus Elfring @ 2017-05-22 14:21 UTC (permalink / raw)
  To: Stefan Hajnoczi
  Cc: kvm, Michael S. Tsirkin, netdev, kernel-janitors, LKML,
	virtualization
In-Reply-To: <20170522140935.GB1021@stefanha-x1.localdomain>

> No objection from me but please make sure to keep vq_err().

How long should I wait before I may dare to send another variant for the
discussed update suggestion?

Which commit message would be acceptable then for this update step?

Regards,
Markus

^ permalink raw reply

* network performance degradation in virtio_net in 4.12-rc
From: Mikulas Patocka @ 2017-05-22 14:25 UTC (permalink / raw)
  To: Michael S. Tsirkin, Jason Wang; +Cc: netdev, kvm, virtualization

Hi

I see severe network performance degradation with the kernels 4.12-rc1 and 
4.12-rc2 in the network virtio driver. Download rate drops down to about 
100kB/s.

I bisected it and it is caused by patch 
d85b758f72b05a774045545f24d70980e3e9aac4 ("virtio_net: fix support for 
small rings"). When I revert this patch, the problem goes away.

The host is Debian Jessie with kernel 4.4.62, the guest is Debian Sid with 
kernel 4.12-rc.

Mikulas

^ permalink raw reply

* Re: network performance degradation in virtio_net in 4.12-rc
From: Michael S. Tsirkin @ 2017-05-22 17:42 UTC (permalink / raw)
  To: Mikulas Patocka; +Cc: netdev, kvm, virtualization
In-Reply-To: <alpine.LRH.2.02.1705221003450.20076@file01.intranet.prod.int.rdu2.redhat.com>

On Mon, May 22, 2017 at 10:25:19AM -0400, Mikulas Patocka wrote:
> Hi
> 
> I see severe network performance degradation with the kernels 4.12-rc1 and 
> 4.12-rc2 in the network virtio driver. Download rate drops down to about 
> 100kB/s.
> 
> I bisected it and it is caused by patch 
> d85b758f72b05a774045545f24d70980e3e9aac4 ("virtio_net: fix support for 
> small rings"). When I revert this patch, the problem goes away.
> 
> The host is Debian Jessie with kernel 4.4.62, the guest is Debian Sid with 
> kernel 4.12-rc.
> 
> Mikulas

Thanks for the report!
I'm not sure what causes it on this host. I'll try to reproduce.
Meanwhile, could you please locate this line of code:
+               vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);

and add something like
	printk(KERN_ERR, "min buf = 0x%x expected 0x%x size 0x%x big %d\n",
	       vi->rq[i].min_buf_len, GOOD_PACKET_LEN,
	       virtqueue_get_vring_size(vi->rq[i].vq),
	       (int)vi->big_packets);

after it?
Then boot and capture the output.

Thanks!

-- 
MST

^ permalink raw reply

* Re: [PATCH 00/10] paravirt: make amount of paravirtualization configurable
From: Boris Ostrovsky @ 2017-05-22 19:42 UTC (permalink / raw)
  To: Juergen Gross, linux-kernel, xen-devel, x86, virtualization
  Cc: jeremy, chrisw, mingo, hpa, tglx, akataria
In-Reply-To: <20170519154746.29389-1-jgross@suse.com>


>  49 files changed, 1548 insertions(+), 1477 deletions(-)
>  create mode 100644 arch/x86/include/asm/paravirt_full.h
>  create mode 100644 arch/x86/include/asm/paravirt_types_full.h
>  create mode 100644 arch/x86/kernel/paravirt_full.c


Do you have this in a tree that can be pulled?

-boris

^ permalink raw reply

* Re: network performance degradation in virtio_net in 4.12-rc
From: Mikulas Patocka @ 2017-05-22 21:50 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, kvm, virtualization
In-Reply-To: <20170522203532-mutt-send-email-mst@kernel.org>



On Mon, 22 May 2017, Michael S. Tsirkin wrote:

> On Mon, May 22, 2017 at 10:25:19AM -0400, Mikulas Patocka wrote:
> > Hi
> > 
> > I see severe network performance degradation with the kernels 4.12-rc1 and 
> > 4.12-rc2 in the network virtio driver. Download rate drops down to about 
> > 100kB/s.
> > 
> > I bisected it and it is caused by patch 
> > d85b758f72b05a774045545f24d70980e3e9aac4 ("virtio_net: fix support for 
> > small rings"). When I revert this patch, the problem goes away.
> > 
> > The host is Debian Jessie with kernel 4.4.62, the guest is Debian Sid with 
> > kernel 4.12-rc.
> > 
> > Mikulas
> 
> Thanks for the report!
> I'm not sure what causes it on this host. I'll try to reproduce.
> Meanwhile, could you please locate this line of code:
> +               vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);
> 
> and add something like
> 	printk(KERN_ERR, "min buf = 0x%x expected 0x%x size 0x%x big %d\n",
> 	       vi->rq[i].min_buf_len, GOOD_PACKET_LEN,
> 	       virtqueue_get_vring_size(vi->rq[i].vq),
> 	       (int)vi->big_packets);
> 
> after it?
> Then boot and capture the output.
> 
> Thanks!

It prints one line "min buf = 0x101 expected 0x5ee size 0x100 big 1".

Mikulas

^ permalink raw reply

* Re: [PATCH 00/10] paravirt: make amount of paravirtualization configurable
From: Juergen Gross @ 2017-05-23  6:27 UTC (permalink / raw)
  To: Boris Ostrovsky, linux-kernel, xen-devel, x86, virtualization
  Cc: jeremy, chrisw, mingo, hpa, tglx, akataria
In-Reply-To: <0bd6745b-9483-4d95-53d5-f13288f4c629@oracle.com>

On 22/05/17 21:42, Boris Ostrovsky wrote:
> 
>>  49 files changed, 1548 insertions(+), 1477 deletions(-)
>>  create mode 100644 arch/x86/include/asm/paravirt_full.h
>>  create mode 100644 arch/x86/include/asm/paravirt_types_full.h
>>  create mode 100644 arch/x86/kernel/paravirt_full.c
> 
> 
> Do you have this in a tree that can be pulled?

https://github.com/jgross1/linux pvops


Juergen

^ permalink raw reply

* [PATCH 02/37] drm: Remove drm_device->virtdev
From: Daniel Vetter @ 2017-05-24 14:51 UTC (permalink / raw)
  To: DRI Development
  Cc: David Airlie, Daniel Vetter, Intel Graphics Development,
	virtualization, Daniel Vetter
In-Reply-To: <20170524145212.27837-1-daniel.vetter@ffwll.ch>

This is a leftover from the drm_bus days, where we've had a
bus-specific device type for every bus type in drm_device. Except for
pci (which we can't remove because dri1 drivers) this is all gone. And
the virt driver also doesn't really need it, dev_to_virtio works
perfectly fine.

Cc: David Airlie <airlied@linux.ie>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 1 -
 drivers/gpu/drm/virtio/virtgpu_kms.c     | 4 ++--
 include/drm/drmP.h                       | 2 --
 3 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
index 43e1d5916c6c..7df8d0c9026a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
@@ -56,7 +56,6 @@ int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev)
 	dev = drm_dev_alloc(driver, &vdev->dev);
 	if (IS_ERR(dev))
 		return PTR_ERR(dev);
-	dev->virtdev = vdev;
 	vdev->priv = dev;
 
 	if (strcmp(vdev->dev.parent->bus->name, "pci") == 0) {
diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
index 1e1c90b30d4a..6400506a06b0 100644
--- a/drivers/gpu/drm/virtio/virtgpu_kms.c
+++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
@@ -138,7 +138,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
 	u32 num_scanouts, num_capsets;
 	int ret;
 
-	if (!virtio_has_feature(dev->virtdev, VIRTIO_F_VERSION_1))
+	if (!virtio_has_feature(dev_to_virtio(dev->dev), VIRTIO_F_VERSION_1))
 		return -ENODEV;
 
 	vgdev = kzalloc(sizeof(struct virtio_gpu_device), GFP_KERNEL);
@@ -147,7 +147,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
 
 	vgdev->ddev = dev;
 	dev->dev_private = vgdev;
-	vgdev->vdev = dev->virtdev;
+	vgdev->vdev = dev_to_virtio(dev->dev);
 	vgdev->dev = dev->dev;
 
 	spin_lock_init(&vgdev->display_info_lock);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 575b29b47811..c363f2fdff31 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -412,8 +412,6 @@ struct drm_device {
 	struct pci_controller *hose;
 #endif
 
-	struct virtio_device *virtdev;
-
 	struct drm_sg_mem *sg;	/**< Scatter gather memory */
 	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
 
-- 
2.11.0

^ permalink raw reply related

* Re: [PATCH 05/10] paravirt: add new PARAVIRT_FULL config item
From: Boris Ostrovsky @ 2017-05-24 15:40 UTC (permalink / raw)
  To: Juergen Gross, linux-kernel, xen-devel, x86, virtualization
  Cc: jeremy, chrisw, mingo, hpa, tglx, akataria
In-Reply-To: <20170519154746.29389-6-jgross@suse.com>

On 05/19/2017 11:47 AM, Juergen Gross wrote:
> Add a new config item PARAVIRT_FULL. It will be used to guard the
> pv_*_ops functions used by fully paravirtualized guests (Xen pv-guests
> and lguest) only.
>
> Kernels not meant to support those guest types will be able to use many
> operations without paravirt abstraction while still supporting all the
> other paravirt features.
>
> For now just add the new Kconfig option and select it for XEN_PV and
> LGUEST_GUEST. Add paravirt_full.c, paravirt_full.h and
> paravirt_types_full.h which will contain the necessary implementation
> parts of the pv guest specific paravirt functions.

Is it not possible to just 'ifdef CONFIG_PARAVIT_FULL' the (ir)relevant
parts of paravirt.[ch] and paravirt_types.c?

Separating structures and files into pv and pvfull seems somewhat
arbitrary (.flush_tlb_others in patch 8 being a good example of one type
of guest deciding to use something that normally would be considered
part of a pvfull-type structure).

-boris

^ permalink raw reply

* Re: [PATCH 05/10] paravirt: add new PARAVIRT_FULL config item
From: Juergen Gross @ 2017-05-24 16:39 UTC (permalink / raw)
  To: Boris Ostrovsky, linux-kernel, xen-devel, x86, virtualization
  Cc: jeremy, chrisw, mingo, hpa, tglx, akataria
In-Reply-To: <260e7bc9-8cba-bc57-9fe8-0e437562229a@oracle.com>

On 24/05/17 17:40, Boris Ostrovsky wrote:
> On 05/19/2017 11:47 AM, Juergen Gross wrote:
>> Add a new config item PARAVIRT_FULL. It will be used to guard the
>> pv_*_ops functions used by fully paravirtualized guests (Xen pv-guests
>> and lguest) only.
>>
>> Kernels not meant to support those guest types will be able to use many
>> operations without paravirt abstraction while still supporting all the
>> other paravirt features.
>>
>> For now just add the new Kconfig option and select it for XEN_PV and
>> LGUEST_GUEST. Add paravirt_full.c, paravirt_full.h and
>> paravirt_types_full.h which will contain the necessary implementation
>> parts of the pv guest specific paravirt functions.
> 
> Is it not possible to just 'ifdef CONFIG_PARAVIT_FULL' the (ir)relevant
> parts of paravirt.[ch] and paravirt_types.c?

Sure it is possible.

The question is whether we want it. This would be a lot of ifdeffery.

The main reason I did it this way was to have a clear split between the
two levels of paravirtualization. A kernel built without pv-full would
not need to include paravirt[_types]_full.h saving some compilation
time (there are lots of source files which are including the paravirt
header now).

> Separating structures and files into pv and pvfull seems somewhat
> arbitrary (.flush_tlb_others in patch 8 being a good example of one type
> of guest deciding to use something that normally would be considered
> part of a pvfull-type structure).

I was thinking of using that for Xen HVM-guests, too. This should speed
up multi-vcpu guests quite a bit.

In case others think doing it via idefs only would be better I'm ready
to change the patches accordingly.


Juergen

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-26 15:09 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-nfs@vger.kernel.org, virtualization,
	anna.schumaker@netapp.com, Trond Myklebust,
	jlayton@poochiereds.net, hch
In-Reply-To: <20170526150839.GA4593@fieldses.org>

Probably should have cc'd virtualization@lists.linux-foundation.org too.

On Fri, May 26, 2017 at 11:08:39AM -0400, bfields@fieldses.org wrote:
> On Tue, May 23, 2017 at 08:23:34AM -0400, bfields@fieldses.org wrote:
> > Unfortunately I can't get anything through testing.  It's not your
> > patches, it's something in -rc1.  My server VM stops responding to
> > any network traffic randomly in the middle of a run.  If I log in from a
> > serial console, I see the interface is up and everything looks OK.  I
> > haven't had the chance to do much more, and I'm not sure where to
> > start....  I started a git-bisect attempt, but there are several
> > unrelated problems, and I'm not sure this one is 100% reproduceable.
> 
> It looks like it may be due to something pulled in with virtio updates.
> I've reproduced the problem on c8b0d7290657 "s390/virtio: change
> maintainership" but not on v4.11.  Are there any known issues with those
> commits?
> 
> I've just been doing this long-running bisect while working on other
> stuff.  My reproducer (basically just running a bunch of NFS
> connectathon tests over a variety of protocol versions and security
> flavors) doesn't hit the bug reliably, and I've had to restart a couple
> times probably due to false negatives.  But this looks pretty promising,
> and there's only 17 commits in that range, so I'll keep bisecting.
> 
> --b.

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-26 19:31 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-nfs@vger.kernel.org, virtualization,
	anna.schumaker@netapp.com, Trond Myklebust,
	jlayton@poochiereds.net, hch
In-Reply-To: <20170526150956.GB4593@fieldses.org>

Looks like the culprit is very likely d85b758f72b0 "virtio_net: fix
support for small rings".

After that patch, my NFS server VM stops responding to packets after a
few minutes of testing.  Before that patch, my server keeps working.

--b.

^ permalink raw reply

* Re: [PATCH 02/37] drm: Remove drm_device->virtdev
From: Gerd Hoffmann @ 2017-05-29  6:52 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: David Airlie, Daniel Vetter, Intel Graphics Development,
	virtualization
In-Reply-To: <20170524145212.27837-3-daniel.vetter@ffwll.ch>

On Wed, 2017-05-24 at 16:51 +0200, Daniel Vetter wrote:
> This is a leftover from the drm_bus days, where we've had a
> bus-specific device type for every bus type in drm_device. Except for
> pci (which we can't remove because dri1 drivers) this is all gone.
> And
> the virt driver also doesn't really need it, dev_to_virtio works
> perfectly fine.

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

^ permalink raw reply

* CFP SENSORNETS 2018 - 6th Int.l Conf. on Sensor Networks (Funchal, Madeira/Portugal)
From: sensornets @ 2017-05-29 17:55 UTC (permalink / raw)
  To: virtualization

SUBMISSION DEADLINE 

6th International Conference on Sensor Networks

Submission Deadline: July 31, 2017

http://www.sensornets.org/

January 22 - 24, 2018
Funchal, Madeira, Portugal.

 SENSORNETS is organized in 5 major tracks:

 - Sensor Networks Software, Architectures and Applications
 - Wireless Sensor Networks
 - Energy and Environment
 - Intelligent Data Analysis and Processing
 - Security and Privacy in Sensor Networks

In Cooperation with IFSA.
 
With the presence of internationally distinguished keynote speakers:
 

A short list of presented papers will be selected so that revised and extended versions of these papers will be published by Springer.
 
All papers presented at the congress venue will also be available at the SCITEPRESS Digital Library (http://www.scitepress.org/DigitalLibrary/).
  
Should you have any question please don’t hesitate contacting me.
 

Kind regards,
SENSORNETS Secretariat

Address: Av. D. Manuel I, 27A, 2º esq.
2910-595 Setubal, Portugal
Tel: +351 265 520 184
Fax: +351 265 520 186
Web: http://www.sensornets.org/
e-mail: sensornets.secretariat@insticc.org

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH 02/37] drm: Remove drm_device->virtdev
From: Neil Armstrong @ 2017-05-30  7:33 UTC (permalink / raw)
  To: Daniel Vetter, DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, virtualization
In-Reply-To: <20170524145212.27837-3-daniel.vetter@ffwll.ch>

On 05/24/2017 04:51 PM, Daniel Vetter wrote:
> This is a leftover from the drm_bus days, where we've had a
> bus-specific device type for every bus type in drm_device. Except for
> pci (which we can't remove because dri1 drivers) this is all gone. And
> the virt driver also doesn't really need it, dev_to_virtio works
> perfectly fine.
> 
> Cc: David Airlie <airlied@linux.ie>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Cc: virtualization@lists.linux-foundation.org
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/virtio/virtgpu_drm_bus.c | 1 -
>  drivers/gpu/drm/virtio/virtgpu_kms.c     | 4 ++--
>  include/drm/drmP.h                       | 2 --
>  3 files changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
> index 43e1d5916c6c..7df8d0c9026a 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drm_bus.c
> @@ -56,7 +56,6 @@ int drm_virtio_init(struct drm_driver *driver, struct virtio_device *vdev)
>  	dev = drm_dev_alloc(driver, &vdev->dev);
>  	if (IS_ERR(dev))
>  		return PTR_ERR(dev);
> -	dev->virtdev = vdev;
>  	vdev->priv = dev;
>  
>  	if (strcmp(vdev->dev.parent->bus->name, "pci") == 0) {
> diff --git a/drivers/gpu/drm/virtio/virtgpu_kms.c b/drivers/gpu/drm/virtio/virtgpu_kms.c
> index 1e1c90b30d4a..6400506a06b0 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_kms.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_kms.c
> @@ -138,7 +138,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>  	u32 num_scanouts, num_capsets;
>  	int ret;
>  
> -	if (!virtio_has_feature(dev->virtdev, VIRTIO_F_VERSION_1))
> +	if (!virtio_has_feature(dev_to_virtio(dev->dev), VIRTIO_F_VERSION_1))
>  		return -ENODEV;
>  
>  	vgdev = kzalloc(sizeof(struct virtio_gpu_device), GFP_KERNEL);
> @@ -147,7 +147,7 @@ int virtio_gpu_driver_load(struct drm_device *dev, unsigned long flags)
>  
>  	vgdev->ddev = dev;
>  	dev->dev_private = vgdev;
> -	vgdev->vdev = dev->virtdev;
> +	vgdev->vdev = dev_to_virtio(dev->dev);
>  	vgdev->dev = dev->dev;
>  
>  	spin_lock_init(&vgdev->display_info_lock);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 575b29b47811..c363f2fdff31 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -412,8 +412,6 @@ struct drm_device {
>  	struct pci_controller *hose;
>  #endif
>  
> -	struct virtio_device *virtdev;
> -
>  	struct drm_sg_mem *sg;	/**< Scatter gather memory */
>  	unsigned int num_crtcs;                  /**< Number of CRTCs on this device */
>  
> 

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: Michael S. Tsirkin @ 2017-05-30 16:26 UTC (permalink / raw)
  To: bfields@fieldses.org
  Cc: linux-nfs@vger.kernel.org, virtualization,
	anna.schumaker@netapp.com, Trond Myklebust,
	jlayton@poochiereds.net, hch
In-Reply-To: <20170526193133.GA9874@fieldses.org>

On Fri, May 26, 2017 at 03:31:33PM -0400, bfields@fieldses.org wrote:
> Looks like the culprit is very likely d85b758f72b0 "virtio_net: fix
> support for small rings".
> 
> After that patch, my NFS server VM stops responding to packets after a
> few minutes of testing.  Before that patch, my server keeps working.
> 
> --b.

Others complained about that too.
I'm still trying to reproduce though.

Meanwhile, could you please locate this line of code:
+               vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);

and add something like
        printk(KERN_ERR, "min buf = 0x%x expected 0x%x size 0x%x big %d\n",
               vi->rq[i].min_buf_len, GOOD_PACKET_LEN,
               virtqueue_get_vring_size(vi->rq[i].vq),
               (int)vi->big_packets);

after it?
Then boot and capture the output.

Thanks!


-- 
MST

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: Michael S. Tsirkin @ 2017-05-30 16:58 UTC (permalink / raw)
  To: bfields@fieldses.org
  Cc: linux-nfs@vger.kernel.org, virtualization,
	anna.schumaker@netapp.com, Trond Myklebust,
	jlayton@poochiereds.net, hch
In-Reply-To: <20170530192544-mutt-send-email-mst@kernel.org>

On Tue, May 30, 2017 at 07:26:37PM +0300, Michael S. Tsirkin wrote:
> On Fri, May 26, 2017 at 03:31:33PM -0400, bfields@fieldses.org wrote:
> > Looks like the culprit is very likely d85b758f72b0 "virtio_net: fix
> > support for small rings".
> > 
> > After that patch, my NFS server VM stops responding to packets after a
> > few minutes of testing.  Before that patch, my server keeps working.
> > 
> > --b.
> 
> Others complained about that too.
> I'm still trying to reproduce though.
> 
> Meanwhile, could you please locate this line of code:
> +               vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);
> 
> and add something like
>         printk(KERN_ERR, "min buf = 0x%x expected 0x%x size 0x%x big %d\n",
>                vi->rq[i].min_buf_len, GOOD_PACKET_LEN,
>                virtqueue_get_vring_size(vi->rq[i].vq),
>                (int)vi->big_packets);
> 
> after it?
> Then boot and capture the output.
> 
> Thanks!
> 

Also, can you pls print the mergeable_rx_buffer_size attribute from
sysfs for this device?


> -- 
> MST

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox