Linux virtualization list
 help / color / mirror / Atom feed
* [PATCH v11 6/6] virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

Add a new vq, cmdq, to handle requests between the device and driver.

This patch implements two commands send from the device and handled in
the driver.
1) cmd VIRTIO_BALLOON_CMDQ_REPORT_STATS: this command is used to report
the guest memory statistics to the host. The stats_vq mechanism is not
used when the cmdq mechanism is enabled.
2) cmd VIRTIO_BALLOON_CMDQ_REPORT_UNUSED_PAGES: this command is used to
report the guest unused pages to the host.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 drivers/virtio/virtio_balloon.c     | 363 ++++++++++++++++++++++++++++++++----
 include/uapi/linux/virtio_balloon.h |  13 ++
 2 files changed, 337 insertions(+), 39 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0cf945c..4ac90a5 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -51,6 +51,10 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
 static struct vfsmount *balloon_mnt;
 #endif
 
+/* Types of pages to chunk */
+#define PAGE_CHUNK_TYPE_BALLOON 0	/* Chunk of inflate/deflate pages */
+#define PAGE_CHNUK_UNUSED_PAGE  1	/* Chunk of unused pages */
+
 /* The size of one page_bmap used to record inflated/deflated pages. */
 #define VIRTIO_BALLOON_PAGE_BMAP_SIZE	(8 * PAGE_SIZE)
 /*
@@ -81,12 +85,25 @@ struct virtio_balloon_page_chunk {
 	unsigned long *page_bmap[VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM];
 };
 
+struct virtio_balloon_cmdq_unused_page {
+	struct virtio_balloon_cmdq_hdr hdr;
+	struct vring_desc *desc_table;
+	/* Number of added descriptors */
+	unsigned int num;
+};
+
+struct virtio_balloon_cmdq_stats {
+	struct virtio_balloon_cmdq_hdr hdr;
+	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+};
+
 struct virtio_balloon {
 	struct virtio_device *vdev;
-	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
+	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq, *cmd_vq;
 
 	/* The balloon servicing is delegated to a freezable workqueue. */
 	struct work_struct update_balloon_stats_work;
+	struct work_struct cmdq_handle_work;
 	struct work_struct update_balloon_size_work;
 
 	/* Prevent updating balloon when it is being canceled. */
@@ -115,8 +132,10 @@ struct virtio_balloon {
 	unsigned int num_pfns;
 	__virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
 
-	/* Memory statistics */
-	struct virtio_balloon_stat stats[VIRTIO_BALLOON_S_NR];
+	/* Cmdq msg buffer for memory statistics */
+	struct virtio_balloon_cmdq_stats cmdq_stats;
+	/* Cmdq msg buffer for reporting ununsed pages */
+	struct virtio_balloon_cmdq_unused_page cmdq_unused_page;
 
 	/* To register callback in oom notifier call chain */
 	struct notifier_block nb;
@@ -208,31 +227,77 @@ static void clear_page_bmap(struct virtio_balloon *vb,
 		       VIRTIO_BALLOON_PAGE_BMAP_SIZE);
 }
 
-static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq)
+static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq,
+			     int type, bool busy_wait)
 {
-	unsigned int len, num;
-	struct vring_desc *desc = vb->balloon_page_chunk.desc_table;
+	unsigned int len, *num, reset_num;
+	struct vring_desc *desc;
+
+	switch (type) {
+	case PAGE_CHUNK_TYPE_BALLOON:
+		desc = vb->balloon_page_chunk.desc_table;
+		num = &vb->balloon_page_chunk.chunk_num;
+		reset_num = 0;
+		break;
+	case PAGE_CHNUK_UNUSED_PAGE:
+		desc = vb->cmdq_unused_page.desc_table;
+		num = &vb->cmdq_unused_page.num;
+		/*
+		 * The first desc is used for the cmdq_hdr, so chunks will be
+		 * added from the second desc.
+		 */
+		reset_num = 1;
+		break;
+	default:
+		dev_warn(&vb->vdev->dev, "%s: unknown page chunk type %d\n",
+			 __func__, type);
+		return;
+	}
 
-	num = vb->balloon_page_chunk.chunk_num;
-	if (!virtqueue_indirect_desc_table_add(vq, desc, num)) {
+	if (!virtqueue_indirect_desc_table_add(vq, desc, *num)) {
 		virtqueue_kick(vq);
-		wait_event(vb->acked, virtqueue_get_buf(vq, &len));
-		vb->balloon_page_chunk.chunk_num = 0;
+		if (busy_wait)
+			while (!virtqueue_get_buf(vq, &len) &&
+			       !virtqueue_is_broken(vq))
+				cpu_relax();
+		else
+			wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+		/*
+		 * Now, the descriptor have been delivered to the host. Reset
+		 * the field in the structure that records the number of added
+		 * descriptors, so that new added descriptor can be re-counted.
+		 */
+		*num = reset_num;
 	}
 }
 
 /* Add a chunk to the buffer. */
 static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
-			  u64 base_addr, u32 size)
+			  int type, u64 base_addr, u32 size)
 {
-	unsigned int *num = &vb->balloon_page_chunk.chunk_num;
-	struct vring_desc *desc = &vb->balloon_page_chunk.desc_table[*num];
+	unsigned int *num;
+	struct vring_desc *desc;
+
+	switch (type) {
+	case PAGE_CHUNK_TYPE_BALLOON:
+		num = &vb->balloon_page_chunk.chunk_num;
+		desc = &vb->balloon_page_chunk.desc_table[*num];
+		break;
+	case PAGE_CHNUK_UNUSED_PAGE:
+		num = &vb->cmdq_unused_page.num;
+		desc = &vb->cmdq_unused_page.desc_table[*num];
+		break;
+	default:
+		dev_warn(&vb->vdev->dev, "%s: chunk %d of unknown pages\n",
+			 __func__, type);
+		return;
+	}
 
 	desc->addr = cpu_to_virtio64(vb->vdev, base_addr);
 	desc->len = cpu_to_virtio32(vb->vdev, size);
 	*num += 1;
 	if (*num == VIRTIO_BALLOON_MAX_PAGE_CHUNKS)
-		send_page_chunks(vb, vq);
+		send_page_chunks(vb, vq, type, false);
 }
 
 static void convert_bmap_to_chunks(struct virtio_balloon *vb,
@@ -264,7 +329,8 @@ static void convert_bmap_to_chunks(struct virtio_balloon *vb,
 		chunk_base_addr = (pfn_start + next_one) <<
 				  VIRTIO_BALLOON_PFN_SHIFT;
 		if (chunk_size) {
-			add_one_chunk(vb, vq, chunk_base_addr, chunk_size);
+			add_one_chunk(vb, vq, PAGE_CHUNK_TYPE_BALLOON,
+				      chunk_base_addr, chunk_size);
 			pos += next_zero + 1;
 		}
 	}
@@ -311,7 +377,7 @@ static void tell_host_from_page_bmap(struct virtio_balloon *vb,
 				       pfn_num);
 	}
 	if (vb->balloon_page_chunk.chunk_num > 0)
-		send_page_chunks(vb, vq);
+		send_page_chunks(vb, vq, PAGE_CHUNK_TYPE_BALLOON, false);
 }
 
 static void set_page_pfns(struct virtio_balloon *vb,
@@ -516,8 +582,8 @@ static inline void update_stat(struct virtio_balloon *vb, int idx,
 			       u16 tag, u64 val)
 {
 	BUG_ON(idx >= VIRTIO_BALLOON_S_NR);
-	vb->stats[idx].tag = cpu_to_virtio16(vb->vdev, tag);
-	vb->stats[idx].val = cpu_to_virtio64(vb->vdev, val);
+	vb->cmdq_stats.stats[idx].tag = cpu_to_virtio16(vb->vdev, tag);
+	vb->cmdq_stats.stats[idx].val = cpu_to_virtio64(vb->vdev, val);
 }
 
 #define pages_to_bytes(x) ((u64)(x) << PAGE_SHIFT)
@@ -582,7 +648,8 @@ static void stats_handle_request(struct virtio_balloon *vb)
 	vq = vb->stats_vq;
 	if (!virtqueue_get_buf(vq, &len))
 		return;
-	sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+	sg_init_one(&sg, vb->cmdq_stats.stats,
+		    sizeof(vb->cmdq_stats.stats[0]) * num_stats);
 	virtqueue_add_outbuf(vq, &sg, 1, vb, GFP_KERNEL);
 	virtqueue_kick(vq);
 }
@@ -686,43 +753,216 @@ static void update_balloon_size_func(struct work_struct *work)
 		queue_work(system_freezable_wq, work);
 }
 
+static void cmdq_handle_stats(struct virtio_balloon *vb)
+{
+	struct scatterlist sg;
+	unsigned int num_stats;
+
+	spin_lock(&vb->stop_update_lock);
+	if (!vb->stop_update) {
+		num_stats = update_balloon_stats(vb);
+		sg_init_one(&sg, &vb->cmdq_stats,
+			    sizeof(struct virtio_balloon_cmdq_hdr) +
+			    sizeof(struct virtio_balloon_stat) * num_stats);
+		virtqueue_add_outbuf(vb->cmd_vq, &sg, 1, vb, GFP_KERNEL);
+		virtqueue_kick(vb->cmd_vq);
+	}
+	spin_unlock(&vb->stop_update_lock);
+}
+
+/*
+ * The header part of the message buffer is given to the device to send a
+ * command to the driver.
+ */
+static void host_cmd_buf_add(struct virtio_balloon *vb,
+			   struct virtio_balloon_cmdq_hdr *hdr)
+{
+	struct scatterlist sg;
+
+	hdr->flags = 0;
+	sg_init_one(&sg, hdr, VIRTIO_BALLOON_CMDQ_HDR_SIZE);
+
+	if (virtqueue_add_inbuf(vb->cmd_vq, &sg, 1, hdr, GFP_KERNEL) < 0) {
+		__virtio_clear_bit(vb->vdev,
+				   VIRTIO_BALLOON_F_CMD_VQ);
+		dev_warn(&vb->vdev->dev, "%s: add miscq msg buf err\n",
+			 __func__);
+		return;
+	}
+
+	virtqueue_kick(vb->cmd_vq);
+}
+
+static void cmdq_handle_unused_pages(struct virtio_balloon *vb)
+{
+	struct virtqueue *vq = vb->cmd_vq;
+	struct vring_desc *hdr_desc = &vb->cmdq_unused_page.desc_table[0];
+	unsigned long hdr_pa;
+	unsigned int order = 0, migratetype = 0;
+	struct zone *zone = NULL;
+	struct page *page = NULL;
+	u64 pfn;
+	int ret = 0;
+
+	/* Put the hdr to the first desc */
+	hdr_pa = virt_to_phys((void *)&vb->cmdq_unused_page.hdr);
+	hdr_desc->addr = cpu_to_virtio64(vb->vdev, hdr_pa);
+	hdr_desc->len = cpu_to_virtio32(vb->vdev,
+				sizeof(struct virtio_balloon_cmdq_hdr));
+	vb->cmdq_unused_page.num = 1;
+
+	for_each_populated_zone(zone) {
+		for (order = MAX_ORDER - 1; order > 0; order--) {
+			for (migratetype = 0; migratetype < MIGRATE_TYPES;
+			     migratetype++) {
+				do {
+					ret = report_unused_page_block(zone,
+						order, migratetype, &page);
+					if (!ret) {
+						pfn = (u64)page_to_pfn(page);
+						add_one_chunk(vb, vq,
+						PAGE_CHNUK_UNUSED_PAGE,
+						pfn << VIRTIO_BALLOON_PFN_SHIFT,
+						(u64)(1 << order) *
+						VIRTIO_BALLOON_PAGES_PER_PAGE);
+					}
+				} while (!ret);
+			}
+		}
+	}
+
+	/* Set the cmd completion flag. */
+	vb->cmdq_unused_page.hdr.flags |=
+				cpu_to_le32(VIRTIO_BALLOON_CMDQ_F_COMPLETION);
+	send_page_chunks(vb, vq, PAGE_CHNUK_UNUSED_PAGE, true);
+}
+
+static void cmdq_handle(struct virtio_balloon *vb)
+{
+	struct virtqueue *vq;
+	struct virtio_balloon_cmdq_hdr *hdr;
+	unsigned int len;
+
+	vq = vb->cmd_vq;
+	while ((hdr = (struct virtio_balloon_cmdq_hdr *)
+			virtqueue_get_buf(vq, &len)) != NULL) {
+		switch (hdr->cmd) {
+		case VIRTIO_BALLOON_CMDQ_REPORT_STATS:
+			cmdq_handle_stats(vb);
+			break;
+		case VIRTIO_BALLOON_CMDQ_REPORT_UNUSED_PAGES:
+			cmdq_handle_unused_pages(vb);
+			break;
+		default:
+			dev_warn(&vb->vdev->dev, "%s: wrong cmd\n", __func__);
+			return;
+		}
+		/*
+		 * Replenish all the command buffer to the device after a
+		 * command is handled. This is for the convenience of the
+		 * device to rewind the cmdq to get back all the command
+		 * buffer after live migration.
+		 */
+		host_cmd_buf_add(vb, &vb->cmdq_stats.hdr);
+		host_cmd_buf_add(vb, &vb->cmdq_unused_page.hdr);
+	}
+}
+
+static void cmdq_handle_work_func(struct work_struct *work)
+{
+	struct virtio_balloon *vb;
+
+	vb = container_of(work, struct virtio_balloon,
+			  cmdq_handle_work);
+	cmdq_handle(vb);
+}
+
+static void cmdq_callback(struct virtqueue *vq)
+{
+	struct virtio_balloon *vb = vq->vdev->priv;
+
+	queue_work(system_freezable_wq, &vb->cmdq_handle_work);
+}
+
 static int init_vqs(struct virtio_balloon *vb)
 {
-	struct virtqueue *vqs[3];
-	vq_callback_t *callbacks[] = { balloon_ack, balloon_ack, stats_request };
-	static const char * const names[] = { "inflate", "deflate", "stats" };
-	int err, nvqs;
+	struct virtqueue **vqs;
+	vq_callback_t **callbacks;
+	const char **names;
+	int err = -ENOMEM;
+	int nvqs;
+
+	/* Inflateq and deflateq are used unconditionally */
+	nvqs = 2;
+
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_CMD_VQ) ||
+	    virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+		nvqs++;
+
+	/* Allocate space for find_vqs parameters */
+	vqs = kcalloc(nvqs, sizeof(*vqs), GFP_KERNEL);
+	if (!vqs)
+		goto err_vq;
+	callbacks = kmalloc_array(nvqs, sizeof(*callbacks), GFP_KERNEL);
+	if (!callbacks)
+		goto err_callback;
+	names = kmalloc_array(nvqs, sizeof(*names), GFP_KERNEL);
+	if (!names)
+		goto err_names;
+
+	callbacks[0] = balloon_ack;
+	names[0] = "inflate";
+	callbacks[1] = balloon_ack;
+	names[1] = "deflate";
 
 	/*
-	 * We expect two virtqueues: inflate and deflate, and
-	 * optionally stat.
+	 * The stats_vq is used only when cmdq is not supported (or disabled)
+	 * by the device.
 	 */
-	nvqs = virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ) ? 3 : 2;
-	err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks, names,
-			NULL);
-	if (err)
-		return err;
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_CMD_VQ)) {
+		callbacks[2] = cmdq_callback;
+		names[2] = "cmdq";
+	} else if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+		callbacks[2] = stats_request;
+		names[2] = "stats";
+	}
 
+	err = vb->vdev->config->find_vqs(vb->vdev, nvqs, vqs, callbacks,
+					 names, NULL);
+	if (err)
+		goto err_find;
 	vb->inflate_vq = vqs[0];
 	vb->deflate_vq = vqs[1];
-	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
+
+	if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_CMD_VQ)) {
+		vb->cmd_vq = vqs[2];
+		/* Prime the cmdq with the header buffer. */
+		host_cmd_buf_add(vb, &vb->cmdq_stats.hdr);
+		host_cmd_buf_add(vb, &vb->cmdq_unused_page.hdr);
+	} else if (virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ)) {
 		struct scatterlist sg;
-		unsigned int num_stats;
-		vb->stats_vq = vqs[2];
 
+		vb->stats_vq = vqs[2];
 		/*
 		 * Prime this virtqueue with one buffer so the hypervisor can
 		 * use it to signal us later (it can't be broken yet!).
 		 */
-		num_stats = update_balloon_stats(vb);
-
-		sg_init_one(&sg, vb->stats, sizeof(vb->stats[0]) * num_stats);
+		sg_init_one(&sg, vb->cmdq_stats.stats,
+			    sizeof(vb->cmdq_stats.stats));
 		if (virtqueue_add_outbuf(vb->stats_vq, &sg, 1, vb, GFP_KERNEL)
 		    < 0)
 			BUG();
 		virtqueue_kick(vb->stats_vq);
 	}
-	return 0;
+
+err_find:
+	kfree(names);
+err_names:
+	kfree(callbacks);
+err_callback:
+	kfree(vqs);
+err_vq:
+	return err;
 }
 
 #ifdef CONFIG_BALLOON_COMPACTION
@@ -730,7 +970,8 @@ static int init_vqs(struct virtio_balloon *vb)
 static void tell_host_one_page(struct virtio_balloon *vb,
 			       struct virtqueue *vq, struct page *page)
 {
-	add_one_chunk(vb, vq, page_to_pfn(page) << VIRTIO_BALLOON_PFN_SHIFT,
+	add_one_chunk(vb, vq, PAGE_CHUNK_TYPE_BALLOON,
+		      page_to_pfn(page) << VIRTIO_BALLOON_PFN_SHIFT,
 		      VIRTIO_BALLOON_PAGES_PER_PAGE);
 }
 
@@ -865,6 +1106,40 @@ static int balloon_page_chunk_init(struct virtio_balloon *vb)
 	return -ENOMEM;
 }
 
+/*
+ * Each type of command is handled one in-flight each time. So, we allocate
+ * one message buffer for each type of command. The header part of the message
+ * buffer will be offered to the device, so that the device can send a command
+ * using the corresponding command buffer to the driver later.
+ */
+static int cmdq_init(struct virtio_balloon *vb)
+{
+	vb->cmdq_unused_page.desc_table = alloc_indirect(vb->vdev,
+						VIRTIO_BALLOON_MAX_PAGE_CHUNKS,
+						GFP_KERNEL);
+	if (!vb->cmdq_unused_page.desc_table) {
+		dev_warn(&vb->vdev->dev, "%s: failed\n", __func__);
+		__virtio_clear_bit(vb->vdev,
+				   VIRTIO_BALLOON_F_CMD_VQ);
+		return -ENOMEM;
+	}
+	vb->cmdq_unused_page.num = 0;
+
+	/*
+	 * The header is initialized to let the device know which type of
+	 * command buffer it receives. The device will later use a buffer
+	 * according to the type of command that it needs to send.
+	 */
+	vb->cmdq_stats.hdr.cmd = VIRTIO_BALLOON_CMDQ_REPORT_STATS;
+	vb->cmdq_stats.hdr.flags = 0;
+	vb->cmdq_unused_page.hdr.cmd = VIRTIO_BALLOON_CMDQ_REPORT_UNUSED_PAGES;
+	vb->cmdq_unused_page.hdr.flags = 0;
+
+	INIT_WORK(&vb->cmdq_handle_work, cmdq_handle_work_func);
+
+	return 0;
+}
+
 static int virtballoon_validate(struct virtio_device *vdev)
 {
 	struct virtio_balloon *vb = NULL;
@@ -883,6 +1158,11 @@ static int virtballoon_validate(struct virtio_device *vdev)
 			goto err_page_chunk;
 	}
 
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_CMD_VQ)) {
+		err = cmdq_init(vb);
+		if (err < 0)
+			goto err_vb;
+	}
 	return 0;
 
 err_page_chunk:
@@ -902,7 +1182,10 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		return -EINVAL;
 	}
 
-	INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
+	if (!virtio_has_feature(vdev, VIRTIO_BALLOON_F_CMD_VQ) &&
+	    virtio_has_feature(vb->vdev, VIRTIO_BALLOON_F_STATS_VQ))
+		INIT_WORK(&vb->update_balloon_stats_work,
+			  update_balloon_stats_func);
 	INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
 	spin_lock_init(&vb->stop_update_lock);
 	vb->stop_update = false;
@@ -980,6 +1263,7 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	spin_unlock_irq(&vb->stop_update_lock);
 	cancel_work_sync(&vb->update_balloon_size_work);
 	cancel_work_sync(&vb->update_balloon_stats_work);
+	cancel_work_sync(&vb->cmdq_handle_work);
 
 	remove_common(vb);
 	free_page_bmap(vb);
@@ -1029,6 +1313,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_STATS_VQ,
 	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
 	VIRTIO_BALLOON_F_PAGE_CHUNKS,
+	VIRTIO_BALLOON_F_CMD_VQ,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 5ed3c7b..cb66c1a 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -35,6 +35,7 @@
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
 #define VIRTIO_BALLOON_F_PAGE_CHUNKS	3 /* Inflate/Deflate pages in chunks */
+#define VIRTIO_BALLOON_F_CMD_VQ		4 /* Command virtqueue */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
@@ -83,4 +84,16 @@ struct virtio_balloon_stat {
 	__virtio64 val;
 } __attribute__((packed));
 
+/* Use the memory of a vring_desc to place the cmdq header */
+#define VIRTIO_BALLOON_CMDQ_HDR_SIZE sizeof(struct vring_desc)
+
+struct virtio_balloon_cmdq_hdr {
+#define VIRTIO_BALLOON_CMDQ_REPORT_STATS	0
+#define VIRTIO_BALLOON_CMDQ_REPORT_UNUSED_PAGES	1
+	__le32 cmd;
+/* Flag to indicate the completion of handling a command */
+#define VIRTIO_BALLOON_CMDQ_F_COMPLETION	1
+	__le32 flags;
+};
+
 #endif /* _LINUX_VIRTIO_BALLOON_H */
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 5/6] mm: export symbol of next_zone and first_online_pgdat
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

This patch enables for_each_zone()/for_each_populated_zone() to be
invoked by a kernel module.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 mm/mmzone.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/mm/mmzone.c b/mm/mmzone.c
index a51c0a6..08a2a3a 100644
--- a/mm/mmzone.c
+++ b/mm/mmzone.c
@@ -13,6 +13,7 @@ struct pglist_data *first_online_pgdat(void)
 {
 	return NODE_DATA(first_online_node);
 }
+EXPORT_SYMBOL_GPL(first_online_pgdat);
 
 struct pglist_data *next_online_pgdat(struct pglist_data *pgdat)
 {
@@ -41,6 +42,7 @@ struct zone *next_zone(struct zone *zone)
 	}
 	return zone;
 }
+EXPORT_SYMBOL_GPL(next_zone);
 
 static inline int zref_in_nodemask(struct zoneref *zref, nodemask_t *nodes)
 {
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 4/6] mm: function to offer a page block on the free list
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

Add a function to find a page block on the free list specified by the
caller. Pages from the page block may be used immediately after the
function returns. The caller is responsible for detecting or preventing
the use of such pages.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
---
 include/linux/mm.h |  5 +++
 mm/page_alloc.c    | 91 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 5d22e69..82361a6 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1841,6 +1841,11 @@ extern void free_area_init_node(int nid, unsigned long * zones_size,
 		unsigned long zone_start_pfn, unsigned long *zholes_size);
 extern void free_initmem(void);
 
+#if IS_ENABLED(CONFIG_VIRTIO_BALLOON)
+extern int report_unused_page_block(struct zone *zone, unsigned int order,
+				    unsigned int migratetype,
+				    struct page **page);
+#endif
 /*
  * Free reserved pages within range [PAGE_ALIGN(start), end & PAGE_MASK)
  * into the buddy system. The freed pages will be poisoned with pattern
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2c25de4..0aefe02 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4615,6 +4615,97 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
 	show_swap_cache_info();
 }
 
+#if IS_ENABLED(CONFIG_VIRTIO_BALLOON)
+
+/*
+ * Heuristically get a page block in the system that is unused.
+ * It is possible that pages from the page block are used immediately after
+ * report_unused_page_block() returns. It is the caller's responsibility
+ * to either detect or prevent the use of such pages.
+ *
+ * The free list to check: zone->free_area[order].free_list[migratetype].
+ *
+ * If the caller supplied page block (i.e. **page) is on the free list, offer
+ * the next page block on the list to the caller. Otherwise, offer the first
+ * page block on the list.
+ *
+ * Return 0 when a page block is found on the caller specified free list.
+ */
+int report_unused_page_block(struct zone *zone, unsigned int order,
+			     unsigned int migratetype, struct page **page)
+{
+	struct zone *this_zone;
+	struct list_head *this_list;
+	int ret = 0;
+	unsigned long flags;
+
+	/* Sanity check */
+	if (zone == NULL || page == NULL || order >= MAX_ORDER ||
+	    migratetype >= MIGRATE_TYPES)
+		return -EINVAL;
+
+	/* Zone validity check */
+	for_each_populated_zone(this_zone) {
+		if (zone == this_zone)
+			break;
+	}
+
+	/* Got a non-existent zone from the caller? */
+	if (zone != this_zone)
+		return -EINVAL;
+
+	spin_lock_irqsave(&this_zone->lock, flags);
+
+	this_list = &zone->free_area[order].free_list[migratetype];
+	if (list_empty(this_list)) {
+		*page = NULL;
+		ret = 1;
+		goto out;
+	}
+
+	/* The caller is asking for the first free page block on the list */
+	if ((*page) == NULL) {
+		*page = list_first_entry(this_list, struct page, lru);
+		ret = 0;
+		goto out;
+	}
+
+	/*
+	 * The page block passed from the caller is not on this free list
+	 * anymore (e.g. a 1MB free page block has been split). In this case,
+	 * offer the first page block on the free list that the caller is
+	 * asking for.
+	 */
+	if (PageBuddy(*page) && order != page_order(*page)) {
+		*page = list_first_entry(this_list, struct page, lru);
+		ret = 0;
+		goto out;
+	}
+
+	/*
+	 * The page block passed from the caller has been the last page block
+	 * on the list.
+	 */
+	if ((*page)->lru.next == this_list) {
+		*page = NULL;
+		ret = 1;
+		goto out;
+	}
+
+	/*
+	 * Finally, fall into the regular case: the page block passed from the
+	 * caller is still on the free list. Offer the next one.
+	 */
+	*page = list_next_entry((*page), lru);
+	ret = 0;
+out:
+	spin_unlock_irqrestore(&this_zone->lock, flags);
+	return ret;
+}
+EXPORT_SYMBOL(report_unused_page_block);
+
+#endif
+
 static void zoneref_set_zone(struct zone *zone, struct zoneref *zoneref)
 {
 	zoneref->zone = zone;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 3/6] virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

Add a new feature, VIRTIO_BALLOON_F_PAGE_CHUNKS, which enables
the transfer of the ballooned (i.e. inflated/deflated) pages in
chunks to the host.

The implementation of the previous virtio-balloon is not very
efficient, because the ballooned pages are transferred to the
host one by one. Here is the breakdown of the time in percentage
spent on each step of the balloon inflating process (inflating
7GB of an 8GB idle guest).

1) allocating pages (6.5%)
2) sending PFNs to host (68.3%)
3) address translation (6.1%)
4) madvise (19%)

It takes about 4126ms for the inflating process to complete.
The above profiling shows that the bottlenecks are stage 2)
and stage 4).

This patch optimizes step 2) by transferring pages to the host in
chunks. A chunk consists of guest physically continuous pages.
When the pages are packed into a chunk, they are converted into
balloon page size (4KB) pages. A chunk is offered to the host
via a base address (i.e. the start guest physical address of those
physically continuous pages) and the size (i.e. the total number
of the 4KB balloon size pages). A chunk is described via a
vring_desc struct in the implementation.

By doing so, step 4) can also be optimized by doing address
translation and madvise() in chunks rather than page by page.

With this new feature, the above ballooning process takes ~590ms
resulting in an improvement of ~85%.

TODO: optimize stage 1) by allocating/freeing a chunk of pages
instead of a single page each time.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
Signed-off-by: Liang Li <liang.z.li@intel.com>
Suggested-by: Michael S. Tsirkin <mst@redhat.com>
---
 drivers/virtio/virtio_balloon.c     | 418 +++++++++++++++++++++++++++++++++---
 drivers/virtio/virtio_ring.c        | 120 ++++++++++-
 include/linux/virtio.h              |   7 +
 include/uapi/linux/virtio_balloon.h |   1 +
 include/uapi/linux/virtio_ring.h    |   3 +
 5 files changed, 517 insertions(+), 32 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index ecb64e9..0cf945c 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -51,6 +51,36 @@ MODULE_PARM_DESC(oom_pages, "pages to free on OOM");
 static struct vfsmount *balloon_mnt;
 #endif
 
+/* The size of one page_bmap used to record inflated/deflated pages. */
+#define VIRTIO_BALLOON_PAGE_BMAP_SIZE	(8 * PAGE_SIZE)
+/*
+ * Callulates how many pfns can a page_bmap record. A bit corresponds to a
+ * page of PAGE_SIZE.
+ */
+#define VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP \
+	(VIRTIO_BALLOON_PAGE_BMAP_SIZE * BITS_PER_BYTE)
+
+/* The number of page_bmap to allocate by default. */
+#define VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM	1
+/* The maximum number of page_bmap that can be allocated. */
+#define VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM	32
+
+/*
+ * QEMU virtio implementation requires the desc table size less than
+ * VIRTQUEUE_MAX_SIZE, so minus 1 here.
+ */
+#define VIRTIO_BALLOON_MAX_PAGE_CHUNKS (VIRTQUEUE_MAX_SIZE - 1)
+
+/* The struct to manage ballooned pages in chunks */
+struct virtio_balloon_page_chunk {
+	/* Indirect desc table to hold chunks of balloon pages */
+	struct vring_desc *desc_table;
+	/* Number of added chunks of balloon pages */
+	unsigned int chunk_num;
+	/* Bitmap used to record ballooned pages. */
+	unsigned long *page_bmap[VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM];
+};
+
 struct virtio_balloon {
 	struct virtio_device *vdev;
 	struct virtqueue *inflate_vq, *deflate_vq, *stats_vq;
@@ -79,6 +109,8 @@ struct virtio_balloon {
 	/* Synchronize access/update to this struct virtio_balloon elements */
 	struct mutex balloon_lock;
 
+	struct virtio_balloon_page_chunk balloon_page_chunk;
+
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
 	__virtio32 pfns[VIRTIO_BALLOON_ARRAY_PFNS_MAX];
@@ -111,6 +143,133 @@ static void balloon_ack(struct virtqueue *vq)
 	wake_up(&vb->acked);
 }
 
+/* Update pfn_max and pfn_min according to the pfn of page */
+static inline void update_pfn_range(struct virtio_balloon *vb,
+				    struct page *page,
+				    unsigned long *pfn_min,
+				    unsigned long *pfn_max)
+{
+	unsigned long pfn = page_to_pfn(page);
+
+	*pfn_min = min(pfn, *pfn_min);
+	*pfn_max = max(pfn, *pfn_max);
+}
+
+static unsigned int extend_page_bmap_size(struct virtio_balloon *vb,
+					  unsigned long pfn_num)
+{
+	unsigned int i, bmap_num, allocated_bmap_num;
+	unsigned long bmap_len;
+
+	allocated_bmap_num = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM;
+	bmap_len = ALIGN(pfn_num, BITS_PER_LONG) / BITS_PER_BYTE;
+	bmap_len = roundup(bmap_len, VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+	/*
+	 * VIRTIO_BALLOON_PAGE_BMAP_SIZE is the size of one page_bmap, so
+	 * divide it to calculate how many page_bmap that we need.
+	 */
+	bmap_num = (unsigned int)(bmap_len / VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+	/* The number of page_bmap to allocate should not exceed the max */
+	bmap_num = min_t(unsigned int, VIRTIO_BALLOON_PAGE_BMAP_MAX_NUM,
+			 bmap_num);
+
+	for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < bmap_num; i++) {
+		vb->balloon_page_chunk.page_bmap[i] =
+			    kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE, GFP_KERNEL);
+		if (vb->balloon_page_chunk.page_bmap[i])
+			allocated_bmap_num++;
+		else
+			break;
+	}
+
+	return allocated_bmap_num;
+}
+
+static void free_extended_page_bmap(struct virtio_balloon *vb,
+				    unsigned int page_bmap_num)
+{
+	unsigned int i;
+
+	for (i = VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i < page_bmap_num;
+	     i++) {
+		kfree(vb->balloon_page_chunk.page_bmap[i]);
+		vb->balloon_page_chunk.page_bmap[i] = NULL;
+		page_bmap_num--;
+	}
+}
+
+static void clear_page_bmap(struct virtio_balloon *vb,
+			    unsigned int page_bmap_num)
+{
+	int i;
+
+	for (i = 0; i < page_bmap_num; i++)
+		memset(vb->balloon_page_chunk.page_bmap[i], 0,
+		       VIRTIO_BALLOON_PAGE_BMAP_SIZE);
+}
+
+static void send_page_chunks(struct virtio_balloon *vb, struct virtqueue *vq)
+{
+	unsigned int len, num;
+	struct vring_desc *desc = vb->balloon_page_chunk.desc_table;
+
+	num = vb->balloon_page_chunk.chunk_num;
+	if (!virtqueue_indirect_desc_table_add(vq, desc, num)) {
+		virtqueue_kick(vq);
+		wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+		vb->balloon_page_chunk.chunk_num = 0;
+	}
+}
+
+/* Add a chunk to the buffer. */
+static void add_one_chunk(struct virtio_balloon *vb, struct virtqueue *vq,
+			  u64 base_addr, u32 size)
+{
+	unsigned int *num = &vb->balloon_page_chunk.chunk_num;
+	struct vring_desc *desc = &vb->balloon_page_chunk.desc_table[*num];
+
+	desc->addr = cpu_to_virtio64(vb->vdev, base_addr);
+	desc->len = cpu_to_virtio32(vb->vdev, size);
+	*num += 1;
+	if (*num == VIRTIO_BALLOON_MAX_PAGE_CHUNKS)
+		send_page_chunks(vb, vq);
+}
+
+static void convert_bmap_to_chunks(struct virtio_balloon *vb,
+				   struct virtqueue *vq,
+				   unsigned long *bmap,
+				   unsigned long pfn_start,
+				   unsigned long size)
+{
+	unsigned long next_one, next_zero, pos = 0;
+	u64 chunk_base_addr;
+	u32 chunk_size;
+
+	while (pos < size) {
+		next_one = find_next_bit(bmap, size, pos);
+		/*
+		 * No "1" bit found, which means that there is no pfn
+		 * recorded in the rest of this bmap.
+		 */
+		if (next_one == size)
+			break;
+		next_zero = find_next_zero_bit(bmap, size, next_one + 1);
+		/*
+		 * A bit in page_bmap corresponds to a page of PAGE_SIZE.
+		 * Convert it to be pages of 4KB balloon page size when
+		 * adding it to a chunk.
+		 */
+		chunk_size = (next_zero - next_one) *
+			     VIRTIO_BALLOON_PAGES_PER_PAGE;
+		chunk_base_addr = (pfn_start + next_one) <<
+				  VIRTIO_BALLOON_PFN_SHIFT;
+		if (chunk_size) {
+			add_one_chunk(vb, vq, chunk_base_addr, chunk_size);
+			pos += next_zero + 1;
+		}
+	}
+}
+
 static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 {
 	struct scatterlist sg;
@@ -124,7 +283,35 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 
 	/* When host has read buffer, this completes via balloon_ack */
 	wait_event(vb->acked, virtqueue_get_buf(vq, &len));
+}
+
+static void tell_host_from_page_bmap(struct virtio_balloon *vb,
+				     struct virtqueue *vq,
+				     unsigned long pfn_start,
+				     unsigned long pfn_end,
+				     unsigned int page_bmap_num)
+{
+	unsigned long i, pfn_num;
 
+	for (i = 0; i < page_bmap_num; i++) {
+		/*
+		 * For the last page_bmap, only the remaining number of pfns
+		 * need to be searched rather than the entire page_bmap.
+		 */
+		if (i + 1 == page_bmap_num)
+			pfn_num = (pfn_end - pfn_start) %
+				  VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+		else
+			pfn_num = VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+
+		convert_bmap_to_chunks(vb, vq,
+				       vb->balloon_page_chunk.page_bmap[i],
+				       pfn_start +
+				       i * VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP,
+				       pfn_num);
+	}
+	if (vb->balloon_page_chunk.chunk_num > 0)
+		send_page_chunks(vb, vq);
 }
 
 static void set_page_pfns(struct virtio_balloon *vb,
@@ -141,13 +328,89 @@ static void set_page_pfns(struct virtio_balloon *vb,
 					  page_to_balloon_pfn(page) + i);
 }
 
+/*
+ * Send ballooned pages in chunks to host.
+ * The ballooned pages are recorded in page bitmaps. Each bit in a bitmap
+ * corresponds to a page of PAGE_SIZE. The page bitmaps are searched for
+ * continuous "1" bits, which correspond to continuous pages, to chunk.
+ * When packing those continuous pages into chunks, pages are converted into
+ * 4KB balloon pages.
+ *
+ * pfn_max and pfn_min form the range of pfns that need to use page bitmaps to
+ * record. If the range is too large to be recorded into the allocated page
+ * bitmaps, the page bitmaps are used multiple times to record the entire
+ * range of pfns.
+ */
+static void tell_host_page_chunks(struct virtio_balloon *vb,
+				  struct list_head *pages,
+				  struct virtqueue *vq,
+				  unsigned long pfn_max,
+				  unsigned long pfn_min)
+{
+	/*
+	 * The pfn_start and pfn_end form the range of pfns that the allocated
+	 * page_bmap can record in each round.
+	 */
+	unsigned long pfn_start, pfn_end;
+	/* Total number of allocated page_bmap */
+	unsigned int page_bmap_num;
+	struct page *page;
+	bool found;
+
+	/*
+	 * In the case that one page_bmap is not sufficient to record the pfn
+	 * range, page_bmap will be extended by allocating more numbers of
+	 * page_bmap.
+	 */
+	page_bmap_num = extend_page_bmap_size(vb, pfn_max - pfn_min + 1);
+
+	/* Start from the beginning of the whole pfn range */
+	pfn_start = pfn_min;
+	while (pfn_start < pfn_max) {
+		pfn_end = pfn_start +
+			  VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP * page_bmap_num;
+		pfn_end = pfn_end < pfn_max ? pfn_end : pfn_max;
+		clear_page_bmap(vb, page_bmap_num);
+		found = false;
+
+		list_for_each_entry(page, pages, lru) {
+			unsigned long bmap_idx, bmap_pos, this_pfn;
+
+			this_pfn = page_to_pfn(page);
+			if (this_pfn < pfn_start || this_pfn > pfn_end)
+				continue;
+			bmap_idx = (this_pfn - pfn_start) /
+				   VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+			bmap_pos = (this_pfn - pfn_start) %
+				   VIRTIO_BALLOON_PFNS_PER_PAGE_BMAP;
+			set_bit(bmap_pos,
+				vb->balloon_page_chunk.page_bmap[bmap_idx]);
+
+			found = true;
+		}
+		if (found)
+			tell_host_from_page_bmap(vb, vq, pfn_start, pfn_end,
+						 page_bmap_num);
+		/*
+		 * Start the next round when pfn_start and pfn_end couldn't
+		 * cover the whole pfn range given by pfn_max and pfn_min.
+		 */
+		pfn_start = pfn_end;
+	}
+	free_extended_page_bmap(vb, page_bmap_num);
+}
+
 static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 {
 	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
 	unsigned num_allocated_pages;
+	bool chunking = virtio_has_feature(vb->vdev,
+					   VIRTIO_BALLOON_F_PAGE_CHUNKS);
+	unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
 
 	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	if (!chunking)
+		num = min(num, ARRAY_SIZE(vb->pfns));
 
 	mutex_lock(&vb->balloon_lock);
 	for (vb->num_pfns = 0; vb->num_pfns < num;
@@ -162,7 +425,10 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 			msleep(200);
 			break;
 		}
-		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		if (chunking)
+			update_pfn_range(vb, page, &pfn_min, &pfn_max);
+		else
+			set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
 		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
 		if (!virtio_has_feature(vb->vdev,
 					VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
@@ -171,8 +437,14 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 
 	num_allocated_pages = vb->num_pfns;
 	/* Did we get any? */
-	if (vb->num_pfns != 0)
-		tell_host(vb, vb->inflate_vq);
+	if (vb->num_pfns != 0) {
+		if (chunking)
+			tell_host_page_chunks(vb, &vb_dev_info->pages,
+					      vb->inflate_vq,
+					      pfn_max, pfn_min);
+		else
+			tell_host(vb, vb->inflate_vq);
+	}
 	mutex_unlock(&vb->balloon_lock);
 
 	return num_allocated_pages;
@@ -198,9 +470,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	struct page *page;
 	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
 	LIST_HEAD(pages);
+	bool chunking = virtio_has_feature(vb->vdev,
+					   VIRTIO_BALLOON_F_PAGE_CHUNKS);
+	unsigned long pfn_max = 0, pfn_min = ULONG_MAX;
 
-	/* We can only do one array worth at a time. */
-	num = min(num, ARRAY_SIZE(vb->pfns));
+	/* Traditionally, we can only do one array worth at a time. */
+	if (!chunking)
+		num = min(num, ARRAY_SIZE(vb->pfns));
 
 	mutex_lock(&vb->balloon_lock);
 	/* We can't release more pages than taken */
@@ -210,7 +486,10 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 		page = balloon_page_dequeue(vb_dev_info);
 		if (!page)
 			break;
-		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		if (chunking)
+			update_pfn_range(vb, page, &pfn_min, &pfn_max);
+		else
+			set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
 		list_add(&page->lru, &pages);
 		vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
 	}
@@ -221,8 +500,13 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	 * virtio_has_feature(vdev, VIRTIO_BALLOON_F_MUST_TELL_HOST);
 	 * is true, we *have* to do it in this order
 	 */
-	if (vb->num_pfns != 0)
-		tell_host(vb, vb->deflate_vq);
+	if (vb->num_pfns != 0) {
+		if (chunking)
+			tell_host_page_chunks(vb, &pages, vb->deflate_vq,
+					      pfn_max, pfn_min);
+		else
+			tell_host(vb, vb->deflate_vq);
+	}
 	release_pages_balloon(vb, &pages);
 	mutex_unlock(&vb->balloon_lock);
 	return num_freed_pages;
@@ -442,6 +726,14 @@ static int init_vqs(struct virtio_balloon *vb)
 }
 
 #ifdef CONFIG_BALLOON_COMPACTION
+
+static void tell_host_one_page(struct virtio_balloon *vb,
+			       struct virtqueue *vq, struct page *page)
+{
+	add_one_chunk(vb, vq, page_to_pfn(page) << VIRTIO_BALLOON_PFN_SHIFT,
+		      VIRTIO_BALLOON_PAGES_PER_PAGE);
+}
+
 /*
  * virtballoon_migratepage - perform the balloon page migration on behalf of
  *			     a compation thread.     (called under page lock)
@@ -465,6 +757,8 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
 {
 	struct virtio_balloon *vb = container_of(vb_dev_info,
 			struct virtio_balloon, vb_dev_info);
+	bool chunking = virtio_has_feature(vb->vdev,
+					   VIRTIO_BALLOON_F_PAGE_CHUNKS);
 	unsigned long flags;
 
 	/*
@@ -486,16 +780,22 @@ static int virtballoon_migratepage(struct balloon_dev_info *vb_dev_info,
 	vb_dev_info->isolated_pages--;
 	__count_vm_event(BALLOON_MIGRATE);
 	spin_unlock_irqrestore(&vb_dev_info->pages_lock, flags);
-	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
-	set_page_pfns(vb, vb->pfns, newpage);
-	tell_host(vb, vb->inflate_vq);
-
+	if (chunking) {
+		tell_host_one_page(vb, vb->inflate_vq, newpage);
+	} else {
+		vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+		set_page_pfns(vb, vb->pfns, newpage);
+		tell_host(vb, vb->inflate_vq);
+	}
 	/* balloon's page migration 2nd step -- deflate "page" */
 	balloon_page_delete(page);
-	vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
-	set_page_pfns(vb, vb->pfns, page);
-	tell_host(vb, vb->deflate_vq);
-
+	if (chunking) {
+		tell_host_one_page(vb, vb->deflate_vq, page);
+	} else {
+		vb->num_pfns = VIRTIO_BALLOON_PAGES_PER_PAGE;
+		set_page_pfns(vb, vb->pfns, page);
+		tell_host(vb, vb->deflate_vq);
+	}
 	mutex_unlock(&vb->balloon_lock);
 
 	put_page(page); /* balloon reference */
@@ -522,9 +822,78 @@ static struct file_system_type balloon_fs = {
 
 #endif /* CONFIG_BALLOON_COMPACTION */
 
+static void free_page_bmap(struct virtio_balloon *vb)
+{
+	int i;
+
+	for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
+		kfree(vb->balloon_page_chunk.page_bmap[i]);
+		vb->balloon_page_chunk.page_bmap[i] = NULL;
+	}
+}
+
+static int balloon_page_chunk_init(struct virtio_balloon *vb)
+{
+	int i;
+
+	vb->balloon_page_chunk.desc_table = alloc_indirect(vb->vdev,
+						VIRTIO_BALLOON_MAX_PAGE_CHUNKS,
+						GFP_KERNEL);
+	if (!vb->balloon_page_chunk.desc_table)
+		goto err_page_chunk;
+	vb->balloon_page_chunk.chunk_num = 0;
+
+	/*
+	 * The default number of page_bmaps are allocated. More may be
+	 * allocated on demand.
+	 */
+	for (i = 0; i < VIRTIO_BALLOON_PAGE_BMAP_DEFAULT_NUM; i++) {
+		vb->balloon_page_chunk.page_bmap[i] =
+			    kmalloc(VIRTIO_BALLOON_PAGE_BMAP_SIZE, GFP_KERNEL);
+		if (!vb->balloon_page_chunk.page_bmap[i])
+			goto err_page_bmap;
+	}
+
+	return 0;
+err_page_bmap:
+	free_page_bmap(vb);
+	kfree(vb->balloon_page_chunk.desc_table);
+	vb->balloon_page_chunk.desc_table = NULL;
+err_page_chunk:
+	__virtio_clear_bit(vb->vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS);
+	dev_warn(&vb->vdev->dev, "%s: failed\n", __func__);
+	return -ENOMEM;
+}
+
+static int virtballoon_validate(struct virtio_device *vdev)
+{
+	struct virtio_balloon *vb = NULL;
+	int err;
+
+	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
+	if (!vb) {
+		err = -ENOMEM;
+		goto err_vb;
+	}
+	vb->vdev = vdev;
+
+	if (virtio_has_feature(vdev, VIRTIO_BALLOON_F_PAGE_CHUNKS)) {
+		err = balloon_page_chunk_init(vb);
+		if (err < 0)
+			goto err_page_chunk;
+	}
+
+	return 0;
+
+err_page_chunk:
+	kfree(vb);
+err_vb:
+	return err;
+}
+
 static int virtballoon_probe(struct virtio_device *vdev)
 {
-	struct virtio_balloon *vb;
+	struct virtio_balloon *vb = vdev->priv;
 	int err;
 
 	if (!vdev->config->get) {
@@ -533,20 +902,14 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		return -EINVAL;
 	}
 
-	vdev->priv = vb = kmalloc(sizeof(*vb), GFP_KERNEL);
-	if (!vb) {
-		err = -ENOMEM;
-		goto out;
-	}
-
 	INIT_WORK(&vb->update_balloon_stats_work, update_balloon_stats_func);
 	INIT_WORK(&vb->update_balloon_size_work, update_balloon_size_func);
 	spin_lock_init(&vb->stop_update_lock);
 	vb->stop_update = false;
 	vb->num_pages = 0;
+
 	mutex_init(&vb->balloon_lock);
 	init_waitqueue_head(&vb->acked);
-	vb->vdev = vdev;
 
 	balloon_devinfo_init(&vb->vb_dev_info);
 
@@ -590,7 +953,6 @@ static int virtballoon_probe(struct virtio_device *vdev)
 	vdev->config->del_vqs(vdev);
 out_free_vb:
 	kfree(vb);
-out:
 	return err;
 }
 
@@ -620,6 +982,8 @@ static void virtballoon_remove(struct virtio_device *vdev)
 	cancel_work_sync(&vb->update_balloon_stats_work);
 
 	remove_common(vb);
+	free_page_bmap(vb);
+	kfree(vb->balloon_page_chunk.desc_table);
 #ifdef CONFIG_BALLOON_COMPACTION
 	if (vb->vb_dev_info.inode)
 		iput(vb->vb_dev_info.inode);
@@ -664,6 +1028,7 @@ static unsigned int features[] = {
 	VIRTIO_BALLOON_F_MUST_TELL_HOST,
 	VIRTIO_BALLOON_F_STATS_VQ,
 	VIRTIO_BALLOON_F_DEFLATE_ON_OOM,
+	VIRTIO_BALLOON_F_PAGE_CHUNKS,
 };
 
 static struct virtio_driver virtio_balloon_driver = {
@@ -674,6 +1039,7 @@ static struct virtio_driver virtio_balloon_driver = {
 	.id_table =	id_table,
 	.probe =	virtballoon_probe,
 	.remove =	virtballoon_remove,
+	.validate =	virtballoon_validate,
 	.config_changed = virtballoon_changed,
 #ifdef CONFIG_PM_SLEEP
 	.freeze	=	virtballoon_freeze,
diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index 409aeaa..0ea2512 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -235,8 +235,17 @@ static int vring_mapping_error(const struct vring_virtqueue *vq,
 	return dma_mapping_error(vring_dma_dev(vq), addr);
 }
 
-static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
-					 unsigned int total_sg, gfp_t gfp)
+/**
+ * alloc_indirect - allocate an indirect desc table
+ * @vdev: the virtio_device that owns the indirect desc table.
+ * @num: the number of entries that the table will have.
+ * @gfp: how to do memory allocations (if necessary).
+ *
+ * Return NULL if the table allocation failed. Otherwise, return the address
+ * of the table.
+ */
+struct vring_desc *alloc_indirect(struct virtio_device *vdev, unsigned int num,
+				  gfp_t gfp)
 {
 	struct vring_desc *desc;
 	unsigned int i;
@@ -248,14 +257,15 @@ static struct vring_desc *alloc_indirect(struct virtqueue *_vq,
 	 */
 	gfp &= ~__GFP_HIGHMEM;
 
-	desc = kmalloc(total_sg * sizeof(struct vring_desc), gfp);
+	desc = kmalloc_array(num, sizeof(struct vring_desc), gfp);
 	if (!desc)
 		return NULL;
 
-	for (i = 0; i < total_sg; i++)
-		desc[i].next = cpu_to_virtio16(_vq->vdev, i + 1);
+	for (i = 0; i < num; i++)
+		desc[i].next = cpu_to_virtio16(vdev, i + 1);
 	return desc;
 }
+EXPORT_SYMBOL_GPL(alloc_indirect);
 
 static inline int virtqueue_add(struct virtqueue *_vq,
 				struct scatterlist *sgs[],
@@ -302,7 +312,7 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 	/* If the host supports indirect descriptor tables, and we have multiple
 	 * buffers, then go indirect. FIXME: tune this threshold */
 	if (vq->indirect && total_sg > 1 && vq->vq.num_free)
-		desc = alloc_indirect(_vq, total_sg, gfp);
+		desc = alloc_indirect(_vq->vdev, total_sg, gfp);
 	else
 		desc = NULL;
 
@@ -433,6 +443,104 @@ static inline int virtqueue_add(struct virtqueue *_vq,
 }
 
 /**
+ * virtqueue_indirect_desc_table_add - add an indirect desc table to the vq
+ * @_vq: the struct virtqueue we're talking about.
+ * @desc: the desc table we're talking about.
+ * @num: the number of entries that the desc table has.
+ *
+ * Returns zero or a negative error (ie. ENOSPC, EIO).
+ */
+int virtqueue_indirect_desc_table_add(struct virtqueue *_vq,
+				      struct vring_desc *desc,
+				      unsigned int num)
+{
+	struct vring_virtqueue *vq = to_vvq(_vq);
+	dma_addr_t desc_addr;
+	unsigned int i, avail;
+	int head;
+
+	/* Sanity check */
+	if (!desc) {
+		pr_debug("%s: empty desc table\n", __func__);
+		return -EINVAL;
+	}
+
+	START_USE(vq);
+
+	if (unlikely(vq->broken)) {
+		END_USE(vq);
+		return -EIO;
+	}
+
+	if (!vq->vq.num_free) {
+		pr_debug("%s: the virtioqueue is full\n", __func__);
+		END_USE(vq);
+		return -ENOSPC;
+	}
+
+	/* Map and fill in the indirect table */
+	desc_addr = vring_map_single(vq, desc, num * sizeof(struct vring_desc),
+				     DMA_TO_DEVICE);
+	if (vring_mapping_error(vq, desc_addr)) {
+		pr_debug("%s: map desc failed\n", __func__);
+		END_USE(vq);
+		return -EIO;
+	}
+
+	/* Mark the flag of the table entries */
+	for (i = 0; i < num; i++)
+		desc[i].flags = cpu_to_virtio16(_vq->vdev, VRING_DESC_F_NEXT);
+	/* The last one doesn't continue. */
+	desc[num - 1].flags &= cpu_to_virtio16(_vq->vdev, ~VRING_DESC_F_NEXT);
+
+	/* Get a ring entry to point to the indirect table */
+	head = vq->free_head;
+	vq->vring.desc[head].flags = cpu_to_virtio16(_vq->vdev,
+						     VRING_DESC_F_INDIRECT);
+	vq->vring.desc[head].addr = cpu_to_virtio64(_vq->vdev, desc_addr);
+	vq->vring.desc[head].len = cpu_to_virtio32(_vq->vdev, num *
+						   sizeof(struct vring_desc));
+	/* We're using 1 buffers from the free list. */
+	vq->vq.num_free--;
+	/* Update free pointer */
+	vq->free_head = virtio16_to_cpu(_vq->vdev, vq->vring.desc[head].next);
+
+	/* Store token and indirect buffer state. */
+	vq->desc_state[head].data = desc;
+	/* Don't free the caller allocated indirect table when detach_buf. */
+	vq->desc_state[head].indir_desc = NULL;
+
+	/*
+	 * Put entry in available array (but don't update avail->idx until they
+	 * do sync).
+	 */
+	avail = vq->avail_idx_shadow & (vq->vring.num - 1);
+	vq->vring.avail->ring[avail] = cpu_to_virtio16(_vq->vdev, head);
+
+	/*
+	 * Descriptors and available array need to be set before we expose the
+	 * new available array entries.
+	 */
+	virtio_wmb(vq->weak_barriers);
+	vq->avail_idx_shadow++;
+	vq->vring.avail->idx = cpu_to_virtio16(_vq->vdev, vq->avail_idx_shadow);
+	vq->num_added++;
+
+	pr_debug("%s: added buffer head %i to %p\n", __func__, head, vq);
+	END_USE(vq);
+
+	/*
+	 * This is very unlikely, but theoretically possible.  Kick
+	 * just in case.
+	 */
+	if (unlikely(vq->num_added == (1 << 16) - 1))
+		virtqueue_kick(_vq);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(virtqueue_indirect_desc_table_add);
+
+/**
  * virtqueue_add_sgs - expose buffers to other end
  * @vq: the struct virtqueue we're talking about.
  * @sgs: array of terminated scatterlists.
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 7edfbdb..01dad22 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -34,6 +34,13 @@ struct virtqueue {
 	void *priv;
 };
 
+struct vring_desc *alloc_indirect(struct virtio_device *vdev,
+				  unsigned int num, gfp_t gfp);
+
+int virtqueue_indirect_desc_table_add(struct virtqueue *_vq,
+				      struct vring_desc *desc,
+				      unsigned int num);
+
 int virtqueue_add_outbuf(struct virtqueue *vq,
 			 struct scatterlist sg[], unsigned int num,
 			 void *data,
diff --git a/include/uapi/linux/virtio_balloon.h b/include/uapi/linux/virtio_balloon.h
index 343d7dd..5ed3c7b 100644
--- a/include/uapi/linux/virtio_balloon.h
+++ b/include/uapi/linux/virtio_balloon.h
@@ -34,6 +34,7 @@
 #define VIRTIO_BALLOON_F_MUST_TELL_HOST	0 /* Tell before reclaiming pages */
 #define VIRTIO_BALLOON_F_STATS_VQ	1 /* Memory Stats virtqueue */
 #define VIRTIO_BALLOON_F_DEFLATE_ON_OOM	2 /* Deflate balloon on OOM */
+#define VIRTIO_BALLOON_F_PAGE_CHUNKS	3 /* Inflate/Deflate pages in chunks */
 
 /* Size of a PFN in the balloon interface. */
 #define VIRTIO_BALLOON_PFN_SHIFT 12
diff --git a/include/uapi/linux/virtio_ring.h b/include/uapi/linux/virtio_ring.h
index c072959..0499fb8 100644
--- a/include/uapi/linux/virtio_ring.h
+++ b/include/uapi/linux/virtio_ring.h
@@ -111,6 +111,9 @@ struct vring {
 #define VRING_USED_ALIGN_SIZE 4
 #define VRING_DESC_ALIGN_SIZE 16
 
+/* The supported max queue size */
+#define VIRTQUEUE_MAX_SIZE 1024
+
 /* The standard layout for the ring is a continuous chunk of memory which looks
  * like this.  We assume num is a power of 2.
  *
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 2/6] virtio-balloon: coding format cleanup
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

Clean up the comment format.

Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 drivers/virtio/virtio_balloon.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 4a9f307..ecb64e9 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -132,8 +132,10 @@ static void set_page_pfns(struct virtio_balloon *vb,
 {
 	unsigned int i;
 
-	/* Set balloon pfns pointing at this page.
-	 * Note that the first pfn points at start of the page. */
+	/*
+	 * Set balloon pfns pointing at this page.
+	 * Note that the first pfn points at start of the page.
+	 */
 	for (i = 0; i < VIRTIO_BALLOON_PAGES_PER_PAGE; i++)
 		pfns[i] = cpu_to_virtio32(vb->vdev,
 					  page_to_balloon_pfn(page) + i);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 1/6] virtio-balloon: deflate via a page list
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource
In-Reply-To: <1497004901-30593-1-git-send-email-wei.w.wang@intel.com>

From: Liang Li <liang.z.li@intel.com>

This patch saves the deflated pages to a list, instead of the PFN array.
Accordingly, the balloon_pfn_to_page() function is removed.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Wei Wang <wei.w.wang@intel.com>
---
 drivers/virtio/virtio_balloon.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 34adf9b..4a9f307 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -104,12 +104,6 @@ static u32 page_to_balloon_pfn(struct page *page)
 	return pfn * VIRTIO_BALLOON_PAGES_PER_PAGE;
 }
 
-static struct page *balloon_pfn_to_page(u32 pfn)
-{
-	BUG_ON(pfn % VIRTIO_BALLOON_PAGES_PER_PAGE);
-	return pfn_to_page(pfn / VIRTIO_BALLOON_PAGES_PER_PAGE);
-}
-
 static void balloon_ack(struct virtqueue *vq)
 {
 	struct virtio_balloon *vb = vq->vdev->priv;
@@ -182,18 +176,16 @@ static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 	return num_allocated_pages;
 }
 
-static void release_pages_balloon(struct virtio_balloon *vb)
+static void release_pages_balloon(struct virtio_balloon *vb,
+				 struct list_head *pages)
 {
-	unsigned int i;
-	struct page *page;
+	struct page *page, *next;
 
-	/* Find pfns pointing at start of each page, get pages and free them. */
-	for (i = 0; i < vb->num_pfns; i += VIRTIO_BALLOON_PAGES_PER_PAGE) {
-		page = balloon_pfn_to_page(virtio32_to_cpu(vb->vdev,
-							   vb->pfns[i]));
+	list_for_each_entry_safe(page, next, pages, lru) {
 		if (!virtio_has_feature(vb->vdev,
 					VIRTIO_BALLOON_F_DEFLATE_ON_OOM))
 			adjust_managed_page_count(page, 1);
+		list_del(&page->lru);
 		put_page(page); /* balloon reference */
 	}
 }
@@ -203,6 +195,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	unsigned num_freed_pages;
 	struct page *page;
 	struct balloon_dev_info *vb_dev_info = &vb->vb_dev_info;
+	LIST_HEAD(pages);
 
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
@@ -216,6 +209,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 		if (!page)
 			break;
 		set_page_pfns(vb, vb->pfns + vb->num_pfns, page);
+		list_add(&page->lru, &pages);
 		vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
 	}
 
@@ -227,7 +221,7 @@ static unsigned leak_balloon(struct virtio_balloon *vb, size_t num)
 	 */
 	if (vb->num_pfns != 0)
 		tell_host(vb, vb->deflate_vq);
-	release_pages_balloon(vb);
+	release_pages_balloon(vb, &pages);
 	mutex_unlock(&vb->balloon_lock);
 	return num_freed_pages;
 }
-- 
2.7.4

^ permalink raw reply related

* [PATCH v11 0/6] Virtio-balloon Enhancement
From: Wei Wang @ 2017-06-09 10:41 UTC (permalink / raw)
  To: virtio-dev, linux-kernel, qemu-devel, virtualization, kvm,
	linux-mm, mst, david, dave.hansen, cornelia.huck, akpm, mgorman,
	aarcange, amit.shah, pbonzini, wei.w.wang, liliang.opensource

This patch series enhances the existing virtio-balloon with the following new
features:
1) fast ballooning: transfer ballooned pages between the guest and host in
chunks, instead of one by one; and
2) cmdq: a new virtqueue to send commands between the device and driver.
Currently, it supports commands to report memory stats (replace the old statq
mechanism) and report guest unused pages.

Liang Li (1):
  virtio-balloon: deflate via a page list

Wei Wang (5):
  virtio-balloon: coding format cleanup
  virtio-balloon: VIRTIO_BALLOON_F_PAGE_CHUNKS
  mm: function to offer a page block on the free list
  mm: export symbol of next_zone and first_online_pgdat
  virtio-balloon: VIRTIO_BALLOON_F_CMD_VQ

 drivers/virtio/virtio_balloon.c     | 781 ++++++++++++++++++++++++++++++++----
 drivers/virtio/virtio_ring.c        | 120 +++++-
 include/linux/mm.h                  |   5 +
 include/linux/virtio.h              |   7 +
 include/uapi/linux/virtio_balloon.h |  14 +
 include/uapi/linux/virtio_ring.h    |   3 +
 mm/mmzone.c                         |   2 +
 mm/page_alloc.c                     |  91 +++++
 8 files changed, 950 insertions(+), 73 deletions(-)

-- 
2.7.4

^ permalink raw reply

* Reminder: CFP: KVM Forum 2017
From: Paolo Bonzini @ 2017-06-08 17:13 UTC (permalink / raw)
  To: KVM list, linux-kernel@vger.kernel.org, Linux Virtualization
In-Reply-To: <fd522779-e417-8372-5ab1-d097c61eaa2d@redhat.com>

Just a quick reminder that the CFP is going to close in a week.

Paolo

On 02/05/2017 12:41, Paolo Bonzini wrote:
> ================================================================
> KVM Forum 2017: Call For Participation
> October 25-27, 2017 - Hilton Prague - Prague, Czech Republic
> 
> (All submissions must be received before midnight June 15, 2017)
> =================================================================
> 
> KVM Forum is an annual event that presents a rare opportunity
> for developers and users to meet, discuss the state of Linux
> virtualization technology, and plan for the challenges ahead. 
> We invite you to lead part of the discussion by submitting a speaking
> proposal for KVM Forum 2017.
> 
> At this highly technical conference, developers driving innovation
> in the KVM virtualization stack (Linux, KVM, QEMU, libvirt) can
> meet users who depend on KVM as part of their offerings, or to
> power their data centers and clouds.
> 
> KVM Forum will include sessions on the state of the KVM
> virtualization stack, planning for the future, and many
> opportunities for attendees to collaborate. As we celebrate ten years
> of KVM development in the Linux kernel, KVM continues to be a
> critical part of the FOSS cloud infrastructure.
> 
> This year, KVM Forum is joining Open Source Summit in Prague, 
> Czech Republic. Selected talks from KVM Forum will be presented on
> Wednesday October 25 to the full audience of the Open Source Summit.
> Also, attendees of KVM Forum will have access to all of the talks from
> Open Source Summit on Wednesday.
> 
> http://events.linuxfoundation.org/cfp
> 
> Suggested topics:
> * Scaling, latency optimizations, performance tuning, real-time guests
> * Hardening and security
> * New features
> * Testing
> 
> KVM and the Linux kernel:
> * Nested virtualization
> * Resource management (CPU, I/O, memory) and scheduling
> * VFIO: IOMMU, SR-IOV, virtual GPU, etc.
> * Networking: Open vSwitch, XDP, etc.
> * virtio and vhost
> * Architecture ports and new processor features
> 
> QEMU:
> * Management interfaces: QOM and QMP
> * New devices, new boards, new architectures
> * Graphics, desktop virtualization and virtual GPU
> * New storage features
> * High availability, live migration and fault tolerance
> * Emulation and TCG
> * Firmware: ACPI, UEFI, coreboot, U-Boot, etc.
> 
> Management and infrastructure
> * Managing KVM: Libvirt, OpenStack, oVirt, etc.
> * Storage: Ceph, Gluster, SPDK, etc.r
> * Network Function Virtualization: DPDK, OPNFV, OVN, etc.
> * Provisioning
> 
> 
> ===============
> SUBMITTING YOUR PROPOSAL
> ===============
> Abstracts due: June 15, 2017
> 
> Please submit a short abstract (~150 words) describing your presentation
> proposal. Slots vary in length up to 45 minutes. Also include the proposal
> type -- one of:
> - technical talk
> - end-user talk
> 
> Submit your proposal here:
> http://events.linuxfoundation.org/cfp
> Please only use the categories "presentation" and "panel discussion"
> 
> You will receive a notification whether or not your presentation proposal
> was accepted by August 10, 2017.
> 
> Speakers will receive a complimentary pass for the event. In the instance
> that case your submission has multiple presenters, only the primary speaker for a
> proposal will receive a complimentary event pass. For panel discussions, all
> panelists will receive a complimentary event pass.
> 
> TECHNICAL TALKS
> 
> A good technical talk should not just report on what has happened over
> the last year; it should present a concrete problem and how it impacts
> the user and/or developer community. Whenever applicable, focus on
> work that needs to be done, difficulties that haven't yet been solved,
> and on decisions that other developers should be aware of. Summarizing
> recent developments is okay but it should not be more than a small
> portion of the overall talk.
> 
> END-USER TALKS
> 
> One of the big challenges as developers is to know what, where and how
> people actually use our software. We will reserve a few slots for end
> users talking about their deployment challenges and achievements.
> 
> If you are using KVM in production you are encouraged submit a speaking
> proposal. Simply mark it as an end-user talk. As an end user, this is a
> unique opportunity to get your input to developers.
> 
> HANDS-ON / BOF SESSIONS
> 
> We will reserve some time for people to get together and discuss
> strategic decisions as well as other topics that are best solved within
> smaller groups.
> 
> These sessions will be announced during the event. If you are interested
> in organizing such a session, please add it to the list at
> 
>   http://www.linux-kvm.org/page/KVM_Forum_2017_BOF
> 
> Let people you think who might be interested know about your BOF, and encourage
> them to add their names to the wiki page as well. Please try to
> add your ideas to the list before KVM Forum starts.
> 
> 
> PANEL DISCUSSIONS
> 
> If you are proposing a panel discussion, please make sure that you list
> all of your potential panelists in your the abstract. We will request full
> biographies if a panel is accepted.
> 
> 
> ===============
> HOTEL / TRAVEL
> ===============
> 
> This year's event will take place at the Hilton Prague.
> For information on discounted room rates for conference attendees
> and on other hotels close to the conference, please visit
> http://events.linuxfoundation.org/events/kvm-forum/attend/hotel-travel.
> 
> ===============
> IMPORTANT DATES
> ===============
> Submission deadline: June 15, 2017
> Notification: August 10, 2017
> Schedule announced: August 17, 2017
> Event dates: October 25-27, 2017
> 
> Thank you for your interest in KVM. We're looking forward to your
> submissions and seeing you at the KVM Forum 2017 in October!
> 
> -your KVM Forum 2017 Program Committee
> 
> Please contact us with any questions or comments at
> kvm-forum-2017-pc@redhat.com
> 

^ permalink raw reply

* Re: BUG: KASAN: use-after-free in free_old_xmit_skbs
From: Michael S. Tsirkin @ 2017-06-05 23:52 UTC (permalink / raw)
  To: Jean-Philippe Menil; +Cc: netdev, John Fastabend, qemu-devel, virtualization
In-Reply-To: <20170605050423-mutt-send-email-mst@kernel.org>

On Mon, Jun 05, 2017 at 05:08:25AM +0300, Michael S. Tsirkin wrote:
> On Mon, Jun 05, 2017 at 12:48:53AM +0200, Jean-Philippe Menil wrote:
> > Hi,
> > 
> > while playing with xdp and ebpf, i'm hitting the following:
> > 
> > [  309.993136]
> > ==================================================================
> > [  309.994735] BUG: KASAN: use-after-free in
> > free_old_xmit_skbs.isra.29+0x2b7/0x2e0 [virtio_net]
> > [  309.998396] Read of size 8 at addr ffff88006aa64220 by task sshd/323
> > [  310.000650]
> > [  310.002305] CPU: 1 PID: 323 Comm: sshd Not tainted 4.12.0-rc3+ #2
> > [  310.004018] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > 1.10.2-20170228_101828-anatol 04/01/2014
> > [  310.006495] Call Trace:
> > [  310.007610]  dump_stack+0xb8/0x14c
> > [  310.008748]  ? _atomic_dec_and_lock+0x174/0x174
> > [  310.009998]  ? pm_qos_get_value.part.7+0x6/0x6
> > [  310.011203]  print_address_description+0x6f/0x280
> > [  310.012416]  kasan_report+0x27a/0x370
> > [  310.013573]  ? free_old_xmit_skbs.isra.29+0x2b7/0x2e0 [virtio_net]
> > [  310.014900]  __asan_report_load8_noabort+0x19/0x20
> > [  310.016136]  free_old_xmit_skbs.isra.29+0x2b7/0x2e0 [virtio_net]
> > [  310.017467]  ? virtnet_del_vqs+0xe0/0xe0 [virtio_net]
> > [  310.018759]  ? packet_rcv+0x20d0/0x20d0
> > [  310.019950]  ? dev_queue_xmit_nit+0x5cd/0xaf0
> > [  310.021168]  start_xmit+0x1b4/0x1b10 [virtio_net]
> > [  310.022413]  ? default_device_exit+0x2d0/0x2d0
> > [  310.023634]  ? virtnet_remove+0xf0/0xf0 [virtio_net]
> > [  310.024874]  ? update_load_avg+0x1281/0x29f0
> > [  310.026059]  dev_hard_start_xmit+0x1ea/0x7f0
> > [  310.027247]  ? validate_xmit_skb_list+0x100/0x100
> > [  310.028470]  ? validate_xmit_skb+0x7f/0xc10
> > [  310.029731]  ? netif_skb_features+0x920/0x920
> > [  310.033469]  ? __skb_tx_hash+0x2f0/0x2f0
> > [  310.035615]  ? validate_xmit_skb_list+0xa3/0x100
> > [  310.037782]  sch_direct_xmit+0x2eb/0x7a0
> > [  310.039842]  ? dev_deactivate_queue.constprop.29+0x230/0x230
> > [  310.041980]  ? netdev_pick_tx+0x212/0x2b0
> > [  310.043868]  __dev_queue_xmit+0x12fa/0x20b0
> > [  310.045564]  ? netdev_pick_tx+0x2b0/0x2b0
> > [  310.047210]  ? __account_cfs_rq_runtime+0x630/0x630
> > [  310.048301]  ? update_stack_state+0x402/0x780
> > [  310.049307]  ? account_entity_enqueue+0x730/0x730
> > [  310.050322]  ? __rb_erase_color+0x27d0/0x27d0
> > [  310.051286]  ? update_curr_fair+0x70/0x70
> > [  310.052206]  ? enqueue_entity+0x2450/0x2450
> > [  310.053124]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.054082]  ? dequeue_entity+0x27a/0x1520
> > [  310.054967]  ? bpf_prog_alloc+0x320/0x320
> > [  310.055822]  ? yield_to_task_fair+0x110/0x110
> > [  310.056708]  ? set_next_entity+0x2f2/0xa90
> > [  310.057574]  ? dequeue_task_fair+0xc09/0x2ec0
> > [  310.058457]  dev_queue_xmit+0x10/0x20
> > [  310.059298]  ip_finish_output2+0xacf/0x12a0
> > [  310.060160]  ? dequeue_entity+0x1520/0x1520
> > [  310.063410]  ? ip_fragment.constprop.47+0x220/0x220
> > [  310.065078]  ? ring_buffer_set_clock+0x50/0x50
> > [  310.066677]  ? __switch_to+0x685/0xda0
> > [  310.068166]  ? load_balance+0x38f0/0x38f0
> > [  310.069544]  ? compat_start_thread+0x80/0x80
> > [  310.070989]  ? trace_find_cmdline+0x60/0x60
> > [  310.072402]  ? rt_cpu_seq_show+0x2d0/0x2d0
> > [  310.073579]  ip_finish_output+0x407/0x880
> > [  310.074441]  ? ip_finish_output+0x407/0x880
> > [  310.075255]  ? update_stack_state+0x402/0x780
> > [  310.076076]  ip_output+0x1c0/0x640
> > [  310.076843]  ? ip_mc_output+0x1350/0x1350
> > [  310.077642]  ? __sk_dst_check+0x164/0x370
> > [  310.078441]  ? complete_formation.isra.53+0xa30/0xa30
> > [  310.079313]  ? __read_once_size_nocheck.constprop.7+0x20/0x20
> > [  310.080265]  ? sock_prot_inuse_add+0xa0/0xa0
> > [  310.081097]  ? memcpy+0x45/0x50
> > [  310.081850]  ? __copy_skb_header+0x1fa/0x280
> > [  310.082676]  ip_local_out+0x70/0x90
> > [  310.083448]  ip_queue_xmit+0x8a1/0x22a0
> > [  310.084236]  ? ip_build_and_send_pkt+0xe80/0xe80
> > [  310.085079]  ? tcp_v4_md5_lookup+0x13/0x20
> > [  310.085884]  tcp_transmit_skb+0x187a/0x3e00
> > [  310.086696]  ? __tcp_select_window+0xaf0/0xaf0
> > [  310.087524]  ? sock_sendmsg+0xba/0xf0
> > [  310.088298]  ? __vfs_write+0x4e0/0x960
> > [  310.089074]  ? vfs_write+0x155/0x4b0
> > [  310.089838]  ? SyS_write+0xf7/0x240
> > [  310.090593]  ? do_syscall_64+0x235/0x5b0
> > [  310.091372]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.094690]  ? sock_sendmsg+0xba/0xf0
> > [  310.096133]  ? do_syscall_64+0x235/0x5b0
> > [  310.097593]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.099157]  ? tcp_init_tso_segs+0x1e0/0x1e0
> > [  310.100539]  ? radix_tree_lookup+0xd/0x10
> > [  310.101894]  ? get_work_pool+0xcd/0x150
> > [  310.103216]  ? check_flush_dependency+0x330/0x330
> > [  310.104113]  tcp_write_xmit+0x498/0x52a0
> > [  310.104905]  ? kasan_unpoison_shadow+0x35/0x50
> > [  310.105729]  ? kasan_kmalloc+0xad/0xe0
> > [  310.106505]  ? tcp_transmit_skb+0x3e00/0x3e00
> > [  310.107331]  ? memset+0x31/0x40
> > [  310.108070]  ? __check_object_size+0x22e/0x55c
> > [  310.108895]  ? skb_pull_rcsum+0x2b0/0x2b0
> > [  310.109690]  ? check_stack_object+0x120/0x120
> > [  310.110512]  ? tcp_v4_md5_lookup+0x13/0x20
> > [  310.111315]  __tcp_push_pending_frames+0x8d/0x2a0
> > [  310.112159]  tcp_push+0x47c/0xbd0
> > [  310.112912]  ? copy_from_iter_full+0x21e/0xc70
> > [  310.113747]  ? sock_warn_obsolete_bsdism+0x70/0x70
> > [  310.114604]  ? tcp_splice_data_recv+0x1c0/0x1c0
> > [  310.115436]  ? iov_iter_copy_from_user_atomic+0xeb0/0xeb0
> > [  310.116324]  tcp_sendmsg+0xd6d/0x43f0
> > [  310.117106]  ? tcp_sendpage+0x2170/0x2170
> > [  310.117911]  ? set_fd_set.part.1+0x50/0x50
> > [  310.118718]  ? remove_wait_queue+0x196/0x3b0
> > [  310.119535]  ? set_fd_set.part.1+0x50/0x50
> > [  310.120365]  ? add_wait_queue_exclusive+0x290/0x290
> > [  310.121224]  ? __wake_up+0x44/0x50
> > [  310.121985]  ? n_tty_read+0x9f9/0x19d0
> > [  310.122898]  ? __check_object_size+0x22e/0x55c
> > [  310.125380]  inet_sendmsg+0x111/0x590
> > [  310.126863]  ? inet_recvmsg+0x5e0/0x5e0
> > [  310.128348]  ? inet_recvmsg+0x5e0/0x5e0
> > [  310.129817]  sock_sendmsg+0xba/0xf0
> > [  310.131110]  sock_write_iter+0x2e4/0x6a0
> > [  310.132433]  ? core_sys_select+0x47d/0x780
> > [  310.133779]  ? sock_sendmsg+0xf0/0xf0
> > [  310.134591]  __vfs_write+0x4e0/0x960
> > [  310.135351]  ? kvm_clock_get_cycles+0x1e/0x20
> > [  310.136160]  ? __vfs_read+0x950/0x950
> > [  310.136931]  ? rw_verify_area+0xbd/0x2b0
> > [  310.137711]  vfs_write+0x155/0x4b0
> > [  310.138454]  SyS_write+0xf7/0x240
> > [  310.139183]  ? SyS_read+0x240/0x240
> > [  310.139922]  ? SyS_read+0x240/0x240
> > [  310.140649]  do_syscall_64+0x235/0x5b0
> > [  310.141390]  ? trace_raw_output_sys_exit+0xf0/0xf0
> > [  310.142204]  ? syscall_return_slowpath+0x240/0x240
> > [  310.143018]  ? trace_do_page_fault+0xc4/0x3a0
> > [  310.143810]  ? prepare_exit_to_usermode+0x124/0x160
> > [  310.144634]  ? perf_trace_sys_enter+0x1080/0x1080
> > [  310.145447]  entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.146257] RIP: 0033:0x7f6f868fb070
> > [  310.146999] RSP: 002b:00007fffed379578 EFLAGS: 00000246 ORIG_RAX:
> > 0000000000000001
> > [  310.148507] RAX: ffffffffffffffda RBX: 00000000000002e4 RCX:
> > 00007f6f868fb070
> > [  310.149521] RDX: 00000000000002e4 RSI: 000055603b5cfc10 RDI:
> > 0000000000000003
> > [  310.150532] RBP: 000055603b5aca60 R08: 0000000000000000 R09:
> > 0000000000003000
> > [  310.151530] R10: 0000000000000008 R11: 0000000000000246 R12:
> > 0000000000000000
> > [  310.152537] R13: 00007fffed37960f R14: 000055603a832e31 R15:
> > 0000000000000003
> > [  310.153578]
> > [  310.156362] Allocated by task 483:
> > [  310.157812]  save_stack_trace+0x1b/0x20
> > [  310.159274]  save_stack+0x43/0xd0
> > [  310.160663]  kasan_kmalloc+0xad/0xe0
> > [  310.161943]  __kmalloc+0x105/0x230
> > [  310.163233]  __vring_new_virtqueue+0xd1/0xee0
> > [  310.164623]  vring_create_virtqueue+0x2e3/0x5e0
> > [  310.165536]  setup_vq+0x136/0x620
> > [  310.166286]  vp_setup_vq+0x13d/0x6d0
> > [  310.167059]  vp_find_vqs_msix+0x46c/0xb50
> > [  310.167855]  vp_find_vqs+0x71/0x410
> > [  310.168641]  vp_modern_find_vqs+0x21/0x140
> > [  310.169453]  init_vqs+0x957/0x1390 [virtio_net]
> > [  310.170306]  virtnet_restore_up+0x4a/0x590 [virtio_net]
> > [  310.171214]  virtnet_xdp+0x89f/0xdf0 [virtio_net]
> > [  310.172077]  dev_change_xdp_fd+0x1ca/0x420
> > [  310.172918]  do_setlink+0x2c33/0x3bc0
> > [  310.173703]  rtnl_setlink+0x245/0x380
> > [  310.174511]  rtnetlink_rcv_msg+0x530/0x9b0
> > [  310.175344]  netlink_rcv_skb+0x213/0x450
> > [  310.176166]  rtnetlink_rcv+0x28/0x30
> > [  310.176990]  netlink_unicast+0x4a0/0x6c0
> > [  310.177807]  netlink_sendmsg+0x9ec/0xe50
> > [  310.178646]  sock_sendmsg+0xba/0xf0
> > [  310.179435]  SYSC_sendto+0x31d/0x620
> > [  310.180229]  SyS_sendto+0xe/0x10
> > [  310.181004]  do_syscall_64+0x235/0x5b0
> > [  310.181783]  return_from_SYSCALL_64+0x0/0x6a
> > [  310.182595]
> > [  310.183217] Freed by task 483:
> > [  310.183934]  save_stack_trace+0x1b/0x20
> > [  310.184801]  save_stack+0x43/0xd0
> > [  310.187187]  kasan_slab_free+0x72/0xc0
> > [  310.188530]  kfree+0x94/0x1a0
> > [  310.189797]  vring_del_virtqueue+0x19a/0x430
> > [  310.191221]  del_vq+0x11c/0x250
> > [  310.192474]  vp_del_vqs+0x379/0xc30
> > [  310.193772]  virtnet_del_vqs+0xad/0xe0 [virtio_net]
> > [  310.195064]  virtnet_xdp+0x836/0xdf0 [virtio_net]
> > [  310.196231]  dev_change_xdp_fd+0x37c/0x420
> > [  310.197072]  do_setlink+0x2c33/0x3bc0
> > [  310.197804]  rtnl_setlink+0x245/0x380
> > [  310.198530]  rtnetlink_rcv_msg+0x530/0x9b0
> > [  310.199283]  netlink_rcv_skb+0x213/0x450
> > [  310.200036]  rtnetlink_rcv+0x28/0x30
> > [  310.200754]  netlink_unicast+0x4a0/0x6c0
> > [  310.201496]  netlink_sendmsg+0x9ec/0xe50
> > [  310.202236]  sock_sendmsg+0xba/0xf0
> > [  310.202947]  SYSC_sendto+0x31d/0x620
> > [  310.203660]  SyS_sendto+0xe/0x10
> > [  310.204340]  do_syscall_64+0x235/0x5b0
> > [  310.205050]  return_from_SYSCALL_64+0x0/0x6a
> > [  310.205792]
> > [  310.206350] The buggy address belongs to the object at ffff88006aa64200
> > [  310.206350]  which belongs to the cache kmalloc-8192 of size 8192
> > [  310.208149] The buggy address is located 32 bytes inside of
> > [  310.208149]  8192-byte region [ffff88006aa64200, ffff88006aa66200)
> > [  310.209929] The buggy address belongs to the page:
> > [  310.210763] page:ffffea0001aa9800 count:1 mapcount:0 mapping:  (null)
> > index:0x0 compound_mapcount: 0
> > [  310.212499] flags: 0x1ffff8000008100(slab|head)
> > [  310.213373] raw: 01ffff8000008100 0000000000000000 0000000000000000
> > 0000000100030003
> > [  310.214481] raw: dead000000000100 dead000000000200 ffff88006cc02700
> > 0000000000000000
> > [  310.215635] page dumped because: kasan: bad access detected
> > [  310.218989]
> > [  310.220398] Memory state around the buggy address:
> > [  310.222141]  ffff88006aa64100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > fc fc
> > [  310.223996]  ffff88006aa64180: fc fc fc fc fc fc fc fc fc fc fc fc fc fc
> > fc fc
> > [  310.225469] >ffff88006aa64200: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > fb fb
> > [  310.227400]                                ^
> > [  310.228367]  ffff88006aa64280: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > fb fb
> > [  310.229510]  ffff88006aa64300: fb fb fb fb fb fb fb fb fb fb fb fb fb fb
> > fb fb
> > [  310.230639]
> > ==================================================================
> > [  310.231788] Disabling lock debugging due to kernel taint
> > [  310.233499] kasan: CONFIG_KASAN_INLINE enabled
> > [  310.236846] kasan: GPF could be caused by NULL-ptr deref or user memory
> > access
> > [  310.239138] general protection fault: 0000 [#1] SMP KASAN
> > [  310.240926] Modules linked in: joydev kvm_intel kvm psmouse irqbypass
> > i2c_piix4 qemu_fw_cfg ip_tables x_tables autofs4 serio_raw virtio_balloon
> > pata_acpi virtio_net virtio_blk
> > [  310.243618] CPU: 0 PID: 352 Comm: sshd Tainted: G    B 4.12.0-rc3+ #2
> > [  310.245780] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> > 1.10.2-20170228_101828-anatol 04/01/2014
> > [  310.249799] task: ffff880066ca8d80 task.stack: ffff880069e40000
> > [  310.251090] RIP: 0010:free_old_xmit_skbs.isra.29+0x9d/0x2e0 [virtio_net]
> > [  310.252403] RSP: 0018:ffff880069e46540 EFLAGS: 00010202
> > [  310.253631] RAX: 0000000000000000 RBX: 0000000000000000 RCX:
> > 0000000000000004
> > [  310.255916] RDX: dffffc0000000000 RSI: 0000000000000008 RDI:
> > 0000000000000020
> > [  310.258017] RBP: ffff880069e465e8 R08: ffff880069e45f10 R09:
> > ffff880066b3c400
> > [  310.259430] R10: ffff880069e45e98 R11: 1ffff1000cd952f3 R12:
> > ffff880066b3c400
> > [  310.260797] R13: ffff880066b3c400 R14: ffff88006afc9156 R15:
> > ffff88006afc9001
> > [  310.262139] FS:  00007f3020f26680(0000) GS:ffff88006d000000(0000)
> > knlGS:0000000000000000
> > [  310.263564] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > [  310.264825] CR2: 00007efed4534010 CR3: 000000006986d000 CR4:
> > 00000000000006f0
> > [  310.266178] Call Trace:
> > [  310.267231]  ? virtnet_del_vqs+0xe0/0xe0 [virtio_net]
> > [  310.268453]  ? packet_rcv+0x20d0/0x20d0
> > [  310.269559]  start_xmit+0x1b4/0x1b10 [virtio_net]
> > [  310.270762]  ? default_device_exit+0x2d0/0x2d0
> > [  310.271910]  ? virtnet_remove+0xf0/0xf0 [virtio_net]
> > [  310.273076]  ? update_load_avg+0x1281/0x29f0
> > [  310.274189]  dev_hard_start_xmit+0x1ea/0x7f0
> > [  310.275295]  ? validate_xmit_skb_list+0x100/0x100
> > [  310.276425]  ? validate_xmit_skb+0x7f/0xc10
> > [  310.277548]  ? rb_insert_color+0x1590/0x1590
> > [  310.280172]  ? netif_skb_features+0x920/0x920
> > [  310.281275]  ? __skb_tx_hash+0x2f0/0x2f0
> > [  310.282362]  ? validate_xmit_skb_list+0xa3/0x100
> > [  310.283494]  sch_direct_xmit+0x2eb/0x7a0
> > [  310.284559]  ? dev_deactivate_queue.constprop.29+0x230/0x230
> > [  310.286448]  ? netdev_pick_tx+0x212/0x2b0
> > [  310.288251]  ? __account_cfs_rq_runtime+0x630/0x630
> > [  310.289707]  __dev_queue_xmit+0x12fa/0x20b0
> > [  310.290788]  ? netdev_pick_tx+0x2b0/0x2b0
> > [  310.291837]  ? update_curr+0x1ef/0x750
> > [  310.292826]  ? update_stack_state+0x402/0x780
> > [  310.293827]  ? account_entity_enqueue+0x730/0x730
> > [  310.294831]  ? update_stack_state+0x402/0x780
> > [  310.295818]  ? update_curr_fair+0x70/0x70
> > [  310.296737]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.297693]  ? dequeue_entity+0x27a/0x1520
> > [  310.298591]  ? bpf_prog_alloc+0x320/0x320
> > [  310.299484]  ? yield_to_task_fair+0x110/0x110
> > [  310.300385]  ? unwind_dump+0x4e0/0x4e0
> > [  310.301246]  ? __free_insn_slot+0x600/0x600
> > [  310.302125]  ? unwind_dump+0x4e0/0x4e0
> > [  310.302975]  ? dequeue_task_fair+0xc09/0x2ec0
> > [  310.303883]  dev_queue_xmit+0x10/0x20
> > [  310.304711]  ip_finish_output2+0xacf/0x12a0
> > [  310.305558]  ? dequeue_entity+0x1520/0x1520
> > [  310.306393]  ? ip_fragment.constprop.47+0x220/0x220
> > [  310.307320]  ? save_stack_trace+0x1b/0x20
> > [  310.308133]  ? save_stack+0x43/0xd0
> > [  310.309081]  ? kasan_slab_free+0x72/0xc0
> > [  310.310614]  ? kfree_skbmem+0xb6/0x1d0
> > [  310.311406]  ? tcp_ack+0x2730/0x7450
> > [  310.312167]  ? tcp_rcv_established+0xdbb/0x2db0
> > [  310.312987]  ? tcp_v4_do_rcv+0x2bb/0x7a0
> > [  310.313769]  ? __release_sock+0x14a/0x2b0
> > [  310.314550]  ? release_sock+0xa8/0x270
> > [  310.315330]  ? inet_sendmsg+0x111/0x590
> > [  310.316100]  ? sock_sendmsg+0xba/0xf0
> > [  310.317403]  ? sock_write_iter+0x2e4/0x6a0
> > [  310.318759]  ? __rb_erase_color+0x27d0/0x27d0
> > [  310.319949]  ? rt_cpu_seq_show+0x2d0/0x2d0
> > [  310.320800]  ? update_stack_state+0x402/0x780
> > [  310.321590]  ip_finish_output+0x407/0x880
> > [  310.322347]  ? ip_finish_output+0x407/0x880
> > [  310.323138]  ? update_stack_state+0x402/0x780
> > [  310.323948]  ip_output+0x1c0/0x640
> > [  310.324661]  ? ip_mc_output+0x1350/0x1350
> > [  310.325415]  ? __sk_dst_check+0x164/0x370
> > [  310.326169]  ? complete_formation.isra.53+0xa30/0xa30
> > [  310.327013]  ? __read_once_size_nocheck.constprop.7+0x20/0x20
> > [  310.327896]  ? sock_prot_inuse_add+0xa0/0xa0
> > [  310.328684]  ? memcpy+0x45/0x50
> > [  310.329393]  ? __copy_skb_header+0x1fa/0x280
> > [  310.330180]  ip_local_out+0x70/0x90
> > [  310.330914]  ip_queue_xmit+0x8a1/0x22a0
> > [  310.331676]  ? ip_build_and_send_pkt+0xe80/0xe80
> > [  310.332517]  ? tcp_v4_md5_lookup+0x13/0x20
> > [  310.333298]  tcp_transmit_skb+0x187a/0x3e00
> > [  310.334085]  ? __tcp_select_window+0xaf0/0xaf0
> > [  310.334887]  ? sock_sendmsg+0xba/0xf0
> > [  310.335637]  ? __vfs_write+0x4e0/0x960
> > [  310.336391]  ? vfs_write+0x155/0x4b0
> > [  310.337135]  ? SyS_write+0xf7/0x240
> > [  310.337861]  ? do_syscall_64+0x235/0x5b0
> > [  310.338612]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.339443]  ? sock_sendmsg+0xba/0xf0
> > [  310.341675]  ? do_syscall_64+0x235/0x5b0
> > [  310.342441]  ? entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.343298]  ? tcp_init_tso_segs+0x1e0/0x1e0
> > [  310.344095]  ? radix_tree_lookup+0xd/0x10
> > [  310.344871]  ? get_work_pool+0xcd/0x150
> > [  310.345635]  ? check_flush_dependency+0x330/0x330
> > [  310.346466]  tcp_write_xmit+0x498/0x52a0
> > [  310.347826]  ? kasan_unpoison_shadow+0x35/0x50
> > [  310.349243]  ? kasan_kmalloc+0xad/0xe0
> > [  310.350156]  ? tcp_transmit_skb+0x3e00/0x3e00
> > [  310.351261]  ? memset+0x31/0x40
> > [  310.352054]  ? __check_object_size+0x22e/0x55c
> > [  310.352881]  ? skb_pull_rcsum+0x2b0/0x2b0
> > [  310.353686]  ? check_stack_object+0x120/0x120
> > [  310.354506]  ? tcp_v4_md5_lookup+0x13/0x20
> > [  310.355327]  __tcp_push_pending_frames+0x8d/0x2a0
> > [  310.356174]  ? tcp_cwnd_restart+0x169/0x440
> > [  310.357016]  tcp_push+0x47c/0xbd0
> > [  310.357777]  ? copy_from_iter_full+0x21e/0xc70
> > [  310.358618]  ? tcp_splice_data_recv+0x1c0/0x1c0
> > [  310.359463]  ? iov_iter_copy_from_user_atomic+0xeb0/0xeb0
> > [  310.360355]  ? tcp_send_mss+0x24/0x2b0
> > [  310.361135]  tcp_sendmsg+0xd6d/0x43f0
> > [  310.361908]  ? select_estimate_accuracy+0x440/0x440
> > [  310.362765]  ? tcp_sendpage+0x2170/0x2170
> > [  310.363583]  ? set_fd_set.part.1+0x50/0x50
> > [  310.364392]  ? remove_wait_queue+0x196/0x3b0
> > [  310.365205]  ? set_fd_set.part.1+0x50/0x50
> > [  310.366005]  ? add_wait_queue_exclusive+0x290/0x290
> > [  310.366865]  ? __wake_up+0x44/0x50
> > [  310.367637]  ? n_tty_read+0x9f9/0x19d0
> > [  310.368424]  ? update_blocked_averages+0x9a0/0x9a0
> > [  310.369283]  ? __check_object_size+0x22e/0x55c
> > [  310.370129]  inet_sendmsg+0x111/0x590
> > [  310.371104]  ? inet_recvmsg+0x5e0/0x5e0
> > [  310.372571]  ? inet_recvmsg+0x5e0/0x5e0
> > [  310.373449]  sock_sendmsg+0xba/0xf0
> > [  310.374217]  sock_write_iter+0x2e4/0x6a0
> > [  310.375005]  ? core_sys_select+0x47d/0x780
> > [  310.375822]  ? sock_sendmsg+0xf0/0xf0
> > [  310.376607]  __vfs_write+0x4e0/0x960
> > [  310.377463]  ? kvm_clock_get_cycles+0x1e/0x20
> > [  310.378864]  ? __vfs_read+0x950/0x950
> > [  310.380178]  ? rw_verify_area+0xbd/0x2b0
> > [  310.381092]  vfs_write+0x155/0x4b0
> > [  310.381877]  SyS_write+0xf7/0x240
> > [  310.382616]  ? SyS_read+0x240/0x240
> > [  310.383404]  ? SyS_read+0x240/0x240
> > [  310.384159]  do_syscall_64+0x235/0x5b0
> > [  310.384930]  ? trace_raw_output_sys_exit+0xf0/0xf0
> > [  310.385747]  ? syscall_return_slowpath+0x240/0x240
> > [  310.386564]  ? trace_do_page_fault+0xc4/0x3a0
> > [  310.387424]  ? prepare_exit_to_usermode+0x124/0x160
> > [  310.388524]  ? perf_trace_sys_enter+0x1080/0x1080
> > [  310.389347]  entry_SYSCALL64_slow_path+0x25/0x25
> > [  310.390164] RIP: 0033:0x7f301f83c070
> > [  310.390906] RSP: 002b:00007ffff738fc78 EFLAGS: 00000246 ORIG_RAX:
> > 0000000000000001
> > [  310.391943] RAX: ffffffffffffffda RBX: 0000000000000564 RCX:
> > 00007f301f83c070
> > [  310.392938] RDX: 0000000000000564 RSI: 000055cf87fb0748 RDI:
> > 0000000000000003
> > [  310.393947] RBP: 000055cf87f8f090 R08: 0000000000000000 R09:
> > 0000000000003000
> > [  310.394948] R10: 0000000000000008 R11: 0000000000000246 R12:
> > 0000000000000000
> > [  310.395967] R13: 00007ffff738fd0f R14: 000055cf873dde31 R15:
> > 0000000000000003
> > [  310.396969] Code: 00 00 48 89 5d d0 31 db 80 3c 02 00 0f 85 05 02 00 00
> > 49 8b 45 00 48 ba 00 00 00 00 00 fc ff df 48 8d 78 20 48 89 f9 48 c1 e9 03
> > <80> 3c 11 00 0f 85 04 02 00 00 48 8b 58 20 48 ba 00 00 00 00 00
> > [  310.399937] RIP: free_old_xmit_skbs.isra.29+0x9d/0x2e0 [virtio_net] RSP:
> > ffff880069e46540
> > [  310.401120] ---[ end trace 89c5b0ea3f07debe ]---
> > [  310.403923] Kernel panic - not syncing: Fatal exception in interrupt
> > [  310.405942] Kernel Offset: 0x33200000 from 0xffffffff81000000 (relocation
> > range: 0xffffffff80000000-0xffffffffbfffffff)
> > [  310.408133] ---[ end Kernel panic - not syncing: Fatal exception in
> > interrupt
> > 
> > 
> > (gdb) l *(free_old_xmit_skbs+0x2b7)
> > 0x22f7 is in free_old_xmit_skbs (drivers/net/virtio_net.c:1051).
> > 1046
> > 1047	static void free_old_xmit_skbs(struct send_queue *sq)
> > 1048	{
> > 1049		struct sk_buff *skb;
> > 1050		unsigned int len;
> > 1051		struct virtnet_info *vi = sq->vq->vdev->priv;
> > 1052		struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
> > 1053		unsigned int packets = 0;
> > 1054		unsigned int bytes = 0;
> > 1055
> > 
> > Let me know if i need to provide more informations.
> > 
> > Best regards.
> > 
> > Jean-Philippe
> 
> So del_vq done during xdp setup seems to race with regular xmit.
> 
> Since commit 680557cf79f82623e2c4fd42733077d60a843513
>     virtio_net: rework mergeable buffer handling
> 
> we no longer must do the resets, we now have enough space
> to store a bit saying whether a buffer is xdp one or not.
> 
> And that's probably a cleaner way to fix these issues than
> try to find and fix the race condition.
> 
> John?
> 
> -- 
> MST


I think I see the source of the race. virtio net calls
netif_device_detach and assumes no packets will be sent after
this point. However, all it does is stop all queues so
no new packets will be transmitted.

Try locking with HARD_TX_LOCK?


-- 
MST

^ permalink raw reply

* Re: [PATCH] virtio_net: lower limit on buffer size
From: J. Bruce Fields @ 2017-06-05 19:36 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Mikulas Patocka
In-Reply-To: <aafb81af-54f2-06b0-3b5c-2713bc28184b@cogentembedded.com>

On Sat, Jun 03, 2017 at 11:17:30PM +0300, Sergei Shtylyov wrote:
> On 06/02/2017 11:25 PM, J. Bruce Fields wrote:
> 
> >>>commit d85b758f72b0 "virtio_net: fix support for small rings"
> >>
> >>   Commit d85b758f72b0 ("virtio_net: fix support for small rings")
> >>
> >>>was supposed to increase the buffer size for small rings
> >>>but had an unintentional side effect of decreasing
> >>>it for large rings. This seems to break some setups -
> >>>it's not yet clear why, but increasing buffer size
> >>>back to what it was before helps.
> >>>
> >>>Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
> >>
> >>Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
> >
> >I may be bikeshedding, but, personally I never do the parens--they're
> >redundant given the quotes, and space is often tight.
> 
>    Just see Documetation/process/submitting-patches.rst.

Yeah, I know, I claim it's a bad rule (but I'm too lazy to send a patch,
so, weight my opinion accordingly).

--b.

^ permalink raw reply

* Re: [PATCH] virtio_net: lower limit on buffer size
From: Sergei Shtylyov @ 2017-06-03 20:17 UTC (permalink / raw)
  To: J. Bruce Fields
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Mikulas Patocka
In-Reply-To: <20170602202536.GA24411@fieldses.org>

On 06/02/2017 11:25 PM, J. Bruce Fields wrote:

>>> commit d85b758f72b0 "virtio_net: fix support for small rings"
>>
>>    Commit d85b758f72b0 ("virtio_net: fix support for small rings")
>>
>>> was supposed to increase the buffer size for small rings
>>> but had an unintentional side effect of decreasing
>>> it for large rings. This seems to break some setups -
>>> it's not yet clear why, but increasing buffer size
>>> back to what it was before helps.
>>>
>>> Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
>>
>> Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
>
> I may be bikeshedding, but, personally I never do the parens--they're
> redundant given the quotes, and space is often tight.

    Just see Documetation/process/submitting-patches.rst.

MBR, Sergei

^ permalink raw reply

* Re: [PATCH] virtio_net: lower limit on buffer size
From: J. Bruce Fields @ 2017-06-02 20:25 UTC (permalink / raw)
  To: Sergei Shtylyov
  Cc: Michael S. Tsirkin, netdev, linux-kernel, virtualization,
	Mikulas Patocka
In-Reply-To: <3dd90d56-7110-bb9e-0b7f-4c8a8003a57f@cogentembedded.com>

On Fri, Jun 02, 2017 at 12:34:57PM +0300, Sergei Shtylyov wrote:
> Hello!
> 
> On 6/2/2017 2:56 AM, Michael S. Tsirkin wrote:
> 
> >commit d85b758f72b0 "virtio_net: fix support for small rings"
> 
>    Commit d85b758f72b0 ("virtio_net: fix support for small rings")
> 
> >was supposed to increase the buffer size for small rings
> >but had an unintentional side effect of decreasing
> >it for large rings. This seems to break some setups -
> >it's not yet clear why, but increasing buffer size
> >back to what it was before helps.
> >
> >Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
> 
> Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")

I may be bikeshedding, but, personally I never do the parens--they're
redundant given the quotes, and space is often tight.

--b.

> 
> >Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> >Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> >Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> [...]
> 
> MBR, Sergei

^ permalink raw reply

* Re: [PATCH v2] virtio_net: lower limit on buffer size
From: David Miller @ 2017-06-02 18:32 UTC (permalink / raw)
  To: mst; +Cc: netdev, linux-kernel, virtualization, bfields, mpatocka
In-Reply-To: <1496415192-17031-1-git-send-email-mst@redhat.com>

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: Fri, 2 Jun 2017 17:54:33 +0300

> commit d85b758f72b0 ("virtio_net: fix support for small rings")
> was supposed to increase the buffer size for small rings but had an
> unintentional side effect of decreasing it for large rings. This seems
> to break some setups - it's not yet clear why, but increasing buffer
> size back to what it was before helps.
> 
> Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> Tested-by: Mikulas Patocka <mpatocka@redhat.com>
> Tested-by: "J. Bruce Fields" <bfields@fieldses.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Patch tested, pls merge.
> 
> changes from v2:
> 	commit log tweaks to address comments by Sergei Shtylyov

Applied, thanks Michael.

^ permalink raw reply

* Re: [bug report] virtio_net: rework mergeable buffer handling
From: Michael S. Tsirkin @ 2017-06-02 15:35 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: virtualization
In-Reply-To: <20170406052949.GA26854@mwanda>

On Thu, Apr 06, 2017 at 08:29:49AM +0300, Dan Carpenter wrote:
> Hello Michael S. Tsirkin,
> 
> The patch 6c8e5f3c41c8: "virtio_net: rework mergeable buffer
> handling" from Mar 6, 2017, leads to the following static checker
> warning:
> 
> 	drivers/net/virtio_net.c:1042 virtnet_receive()
> 	error: uninitialized symbol 'ctx'.
> 
> drivers/net/virtio_net.c
>   1030  static int virtnet_receive(struct receive_queue *rq, int budget)
>   1031  {
>   1032          struct virtnet_info *vi = rq->vq->vdev->priv;
>   1033          unsigned int len, received = 0, bytes = 0;
>   1034          void *buf;
>   1035          struct virtnet_stats *stats = this_cpu_ptr(vi->stats);
>   1036  
>   1037          if (vi->mergeable_rx_bufs) {
>   1038                  void *ctx;
>                               ^^^
>   1039  
>   1040                  while (received < budget &&
>   1041                         (buf = virtqueue_get_buf_ctx(rq->vq, &len, &ctx))) {
>                                                                            ^^^^
>   1042                          bytes += receive_buf(vi, rq, buf, len, ctx);
>                                                                        ^^^
> 
> It's possible that this code is correct, but I looked at it and wasn't
> immediately convinced.  Returning non-NULL buf is not sufficient to
> show that "ctx" is initialized, because if it's vq->indirect then "buf"
> is still unintialized.  Also it's possible that receive_buf() checks
> vq->indirect through some side effect way that I didn't see so it
> doesn't use the uninitialized value...
> 
> I feel like if this is a false positive, that means the rules are too
> subtle...  :/

Yes, false positive I think.

What happens is this: __vring_new_virtqueue does:
        vq->indirect = virtio_has_feature(vdev, VIRTIO_RING_F_INDIRECT_DESC) &&
                !context;

so indirect is never set with context.

Adding code comments should help - could you take a stub at it?


>   1043                          received++;
>   1044                  }
>   1045          } else {
>   1046                  while (received < budget &&
>   1047                         (buf = virtqueue_get_buf(rq->vq, &len)) != NULL) {
>   1048                          bytes += receive_buf(vi, rq, buf, len, NULL);
>   1049                          received++;
>   1050                  }
>   1051          }
>   1052  
> 
> regards,
> dan carpenter

^ permalink raw reply

* [PATCH v2] virtio_net: lower limit on buffer size
From: Michael S. Tsirkin @ 2017-06-02 14:54 UTC (permalink / raw)
  To: linux-kernel, David Miller
  Cc: J. Bruce Fields, netdev, Mikulas Patocka, virtualization

commit d85b758f72b0 ("virtio_net: fix support for small rings")
was supposed to increase the buffer size for small rings but had an
unintentional side effect of decreasing it for large rings. This seems
to break some setups - it's not yet clear why, but increasing buffer
size back to what it was before helps.

Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
Tested-by: Mikulas Patocka <mpatocka@redhat.com>
Tested-by: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Patch tested, pls merge.

changes from v2:
	commit log tweaks to address comments by Sergei Shtylyov

 drivers/net/virtio_net.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 87b5c20..60abb5d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -842,7 +842,7 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
 	unsigned int len;
 
 	len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
-				rq->min_buf_len - hdr_len, PAGE_SIZE - hdr_len);
+				rq->min_buf_len, PAGE_SIZE - hdr_len);
 	return ALIGN(len, L1_CACHE_BYTES);
 }
 
@@ -2039,7 +2039,8 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
 	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
 	unsigned int min_buf_len = DIV_ROUND_UP(buf_len, rq_size);
 
-	return max(min_buf_len, hdr_len);
+	return max(max(min_buf_len, hdr_len) - hdr_len,
+		   (unsigned int)GOOD_PACKET_LEN);
 }
 
 static int virtnet_find_vqs(struct virtnet_info *vi)
-- 
MST

^ permalink raw reply related

* Re: [PATCH] virtio_net: lower limit on buffer size
From: Mikulas Patocka @ 2017-06-02 13:16 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: J. Bruce Fields, netdev, linux-kernel, virtualization
In-Reply-To: <1496361148-4603-1-git-send-email-mst@redhat.com>



On Fri, 2 Jun 2017, Michael S. Tsirkin wrote:

> commit d85b758f72b0 "virtio_net: fix support for small rings"
> was supposed to increase the buffer size for small rings
> but had an unintentional side effect of decreasing
> it for large rings. This seems to break some setups -
> it's not yet clear why, but increasing buffer size
> back to what it was before helps.
> 
> Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Can reporters please confirm whether the following helps?
> If it does I think we should queue it even though I expect I'll need to
> investigate the exact reasons for the failure down the road - probably
> a hypervisor bug.

Hi

This patch fixes the problem for me.

Mikulas

>  drivers/net/virtio_net.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 87b5c20..60abb5d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -842,7 +842,7 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
>  	unsigned int len;
>  
>  	len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
> -				rq->min_buf_len - hdr_len, PAGE_SIZE - hdr_len);
> +				rq->min_buf_len, PAGE_SIZE - hdr_len);
>  	return ALIGN(len, L1_CACHE_BYTES);
>  }
>  
> @@ -2039,7 +2039,8 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
>  	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
>  	unsigned int min_buf_len = DIV_ROUND_UP(buf_len, rq_size);
>  
> -	return max(min_buf_len, hdr_len);
> +	return max(max(min_buf_len, hdr_len) - hdr_len,
> +		   (unsigned int)GOOD_PACKET_LEN);
>  }
>  
>  static int virtnet_find_vqs(struct virtnet_info *vi)
> -- 
> MST
> 

^ permalink raw reply

* Re: [PATCH] virtio_net: lower limit on buffer size
From: Sergei Shtylyov @ 2017-06-02  9:34 UTC (permalink / raw)
  To: Michael S. Tsirkin, linux-kernel
  Cc: J. Bruce Fields, netdev, Mikulas Patocka, virtualization
In-Reply-To: <1496361148-4603-1-git-send-email-mst@redhat.com>

Hello!

On 6/2/2017 2:56 AM, Michael S. Tsirkin wrote:

> commit d85b758f72b0 "virtio_net: fix support for small rings"

    Commit d85b758f72b0 ("virtio_net: fix support for small rings")

> was supposed to increase the buffer size for small rings
> but had an unintentional side effect of decreasing
> it for large rings. This seems to break some setups -
> it's not yet clear why, but increasing buffer size
> back to what it was before helps.
>
> Fixes: d85b758f72b0 "virtio_net: fix support for small rings"

Fixes: d85b758f72b0 ("virtio_net: fix support for small rings")

> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
[...]

MBR, Sergei

^ permalink raw reply

* Re: [PATCH] virtio_net: lower limit on buffer size
From: J. Bruce Fields @ 2017-06-02  1:57 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: netdev, Mikulas Patocka, linux-kernel, virtualization
In-Reply-To: <1496361148-4603-1-git-send-email-mst@redhat.com>

On Fri, Jun 02, 2017 at 02:56:04AM +0300, Michael S. Tsirkin wrote:
> commit d85b758f72b0 "virtio_net: fix support for small rings"
> was supposed to increase the buffer size for small rings
> but had an unintentional side effect of decreasing
> it for large rings. This seems to break some setups -
> it's not yet clear why, but increasing buffer size
> back to what it was before helps.
> 
> Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> Can reporters please confirm whether the following helps?

Works for me.

--b.

> If it does I think we should queue it even though I expect I'll need to
> investigate the exact reasons for the failure down the road - probably
> a hypervisor bug.
> 
>  drivers/net/virtio_net.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 87b5c20..60abb5d 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -842,7 +842,7 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
>  	unsigned int len;
>  
>  	len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
> -				rq->min_buf_len - hdr_len, PAGE_SIZE - hdr_len);
> +				rq->min_buf_len, PAGE_SIZE - hdr_len);
>  	return ALIGN(len, L1_CACHE_BYTES);
>  }
>  
> @@ -2039,7 +2039,8 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
>  	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
>  	unsigned int min_buf_len = DIV_ROUND_UP(buf_len, rq_size);
>  
> -	return max(min_buf_len, hdr_len);
> +	return max(max(min_buf_len, hdr_len) - hdr_len,
> +		   (unsigned int)GOOD_PACKET_LEN);
>  }
>  
>  static int virtnet_find_vqs(struct virtnet_info *vi)
> -- 
> MST

^ permalink raw reply

* [PATCH] virtio_net: lower limit on buffer size
From: Michael S. Tsirkin @ 2017-06-01 23:56 UTC (permalink / raw)
  To: linux-kernel; +Cc: J. Bruce Fields, netdev, Mikulas Patocka, virtualization

commit d85b758f72b0 "virtio_net: fix support for small rings"
was supposed to increase the buffer size for small rings
but had an unintentional side effect of decreasing
it for large rings. This seems to break some setups -
it's not yet clear why, but increasing buffer size
back to what it was before helps.

Fixes: d85b758f72b0 "virtio_net: fix support for small rings"
Reported-by: Mikulas Patocka <mpatocka@redhat.com>
Reported-by: "J. Bruce Fields" <bfields@fieldses.org>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

Can reporters please confirm whether the following helps?
If it does I think we should queue it even though I expect I'll need to
investigate the exact reasons for the failure down the road - probably
a hypervisor bug.

 drivers/net/virtio_net.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 87b5c20..60abb5d 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -842,7 +842,7 @@ static unsigned int get_mergeable_buf_len(struct receive_queue *rq,
 	unsigned int len;
 
 	len = hdr_len + clamp_t(unsigned int, ewma_pkt_len_read(avg_pkt_len),
-				rq->min_buf_len - hdr_len, PAGE_SIZE - hdr_len);
+				rq->min_buf_len, PAGE_SIZE - hdr_len);
 	return ALIGN(len, L1_CACHE_BYTES);
 }
 
@@ -2039,7 +2039,8 @@ static unsigned int mergeable_min_buf_len(struct virtnet_info *vi, struct virtqu
 	unsigned int buf_len = hdr_len + ETH_HLEN + VLAN_HLEN + packet_len;
 	unsigned int min_buf_len = DIV_ROUND_UP(buf_len, rq_size);
 
-	return max(min_buf_len, hdr_len);
+	return max(max(min_buf_len, hdr_len) - hdr_len,
+		   (unsigned int)GOOD_PACKET_LEN);
 }
 
 static int virtnet_find_vqs(struct virtnet_info *vi)
-- 
MST

^ permalink raw reply related

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-31 21:15 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: <20170531210923.GF23526@fieldses.org>

On Wed, May 31, 2017 at 05:09:23PM -0400, bfields@fieldses.org wrote:
> 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.
> 
> Doesn't look like that code's run on boot; apply the below, boot, and:

Whoops, no, just a typo in the printk.  Here you go:

	min buf = 0x101 expected 0x5ee size 0x100 big 1

--b.

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-31 21: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: <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.

Doesn't look like that code's run on boot; apply the below, boot, and:

 $ dmesg|grep expected

gives no output.

--b.

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 9320d96a1632..b10014f7b480 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -2212,6 +2212,10 @@ static int virtnet_find_vqs(struct virtnet_info *vi)
        for (i = 0; i < vi->max_queue_pairs; i++) {
                vi->rq[i].vq = vqs[rxq2vq(i)];
                vi->rq[i].min_buf_len = mergeable_min_buf_len(vi, vi->rq[i].vq);
+               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);
                vi->sq[i].vq = vqs[txq2vq(i)];
        }

^ permalink raw reply related

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-31 21:00 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: <20170530200109-mutt-send-email-mst@kernel.org>

On Tue, May 30, 2017 at 08:03:12PM +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.
> 
> 
> So I think I know what caused this: looks like some hypervisors
> aren't prepared to deal with a situation where packet size
> becomes very small.
> 
> But which hypervisors exactly? I'd like to know in order to detect these
> and decide whether I blacklist bad ones or whitelist known-good ones.

I'm running this under KVM on a Fedora 25 host
(4.10.15-200.fc25.x86_64).  Let me know if any more details about the
setup would be useful.

--b.

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: bfields @ 2017-05-31 20:57 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: <20170530195716-mutt-send-email-mst@kernel.org>

On Tue, May 30, 2017 at 07:58:06PM +0300, Michael S. Tsirkin wrote:
> 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?

On 4.12-rc1:

	# cat
	# /sys/devices/pci0000:00/0000:00:03.0/virtio0/net/eth0/queues/rx-0/virtio_net/mergeable_rx_buffer_size
	320

--b.

^ permalink raw reply

* Re: remove function pointer casts and constify function tables
From: Michael S. Tsirkin @ 2017-05-30 17:03 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.


So I think I know what caused this: looks like some hypervisors
aren't prepared to deal with a situation where packet size
becomes very small.

But which hypervisors exactly? I'd like to know in order to detect these
and decide whether I blacklist bad ones or whitelist known-good ones.

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