All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: rusty@rustcorp.com.au, mst@redhat.com
Cc: virtualization@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, paulus@samba.org,
	qemu-devel@nongnu.org, David Gibson <david@gibson.dropbear.id.au>
Subject: [PATCH 1/3] virtio_balloon: Remove unnecessarily persistent state
Date: Thu, 12 Apr 2012 15:36:33 +1000	[thread overview]
Message-ID: <1334208995-29985-2-git-send-email-david@gibson.dropbear.id.au> (raw)
In-Reply-To: <1334208995-29985-1-git-send-email-david@gibson.dropbear.id.au>

The main virtio_balloon state structure contains the fields num_pfns and
array 'pfns'.  Although they are stored here persistently, the lifetime of
useful data in there is never more than one function - they're essentially
used as though they were local variables.

For the pfns buffer, used to communicate a batch of pfns this is useful to
avoid either transient kmalloc()s or putting too much data on the stack.
For num_pfns, there is no reason it should not be a local, though.

This patch cleans things up by making num_pfns a local in the functions it
is used in.  The pfns array remains, but the comment is updated to clarify
that it contains no truly persistent data.

Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
 drivers/virtio/virtio_balloon.c |   28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 05f0a80..6c07793 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -46,8 +46,8 @@ struct virtio_balloon
 	unsigned int num_pages;
 	struct list_head pages;
 
-	/* The array of pfns we tell the Host about. */
-	unsigned int num_pfns;
+	/* Temporary buffer of pfns to pass to the host */
+	/* Store this here to avoid a too-large local array */
 	u32 pfns[256];
 
 	/* Memory statistics */
@@ -79,11 +79,12 @@ static void balloon_ack(struct virtqueue *vq)
 		complete(&vb->acked);
 }
 
-static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
+static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq,
+		      unsigned int n)
 {
 	struct scatterlist sg;
 
-	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * vb->num_pfns);
+	sg_init_one(&sg, vb->pfns, sizeof(vb->pfns[0]) * n);
 
 	init_completion(&vb->acked);
 
@@ -98,10 +99,12 @@ static void tell_host(struct virtio_balloon *vb, struct virtqueue *vq)
 
 static void fill_balloon(struct virtio_balloon *vb, size_t num)
 {
+	unsigned int n;
+
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
 
-	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
+	for (n = 0; n < num; n++) {
 		struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY |
 					__GFP_NOMEMALLOC | __GFP_NOWARN);
 		if (!page) {
@@ -113,17 +116,17 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
 			msleep(200);
 			break;
 		}
-		vb->pfns[vb->num_pfns] = page_to_balloon_pfn(page);
+		vb->pfns[n] = page_to_balloon_pfn(page);
 		totalram_pages--;
 		vb->num_pages++;
 		list_add(&page->lru, &vb->pages);
 	}
 
 	/* Didn't get any?  Oh well. */
-	if (vb->num_pfns == 0)
+	if (n == 0)
 		return;
 
-	tell_host(vb, vb->inflate_vq);
+	tell_host(vb, vb->inflate_vq, n);
 }
 
 static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
@@ -139,14 +142,15 @@ static void release_pages_by_pfn(const u32 pfns[], unsigned int num)
 static void leak_balloon(struct virtio_balloon *vb, size_t num)
 {
 	struct page *page;
+	unsigned int n;
 
 	/* We can only do one array worth at a time. */
 	num = min(num, ARRAY_SIZE(vb->pfns));
 
-	for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) {
+	for (n = 0; n < num; n++) {
 		page = list_first_entry(&vb->pages, struct page, lru);
 		list_del(&page->lru);
-		vb->pfns[vb->num_pfns] = page_to_balloon_pfn(page);
+		vb->pfns[n] = page_to_balloon_pfn(page);
 		vb->num_pages--;
 	}
 
@@ -155,8 +159,8 @@ static void 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
 	 */
-	tell_host(vb, vb->deflate_vq);
-	release_pages_by_pfn(vb->pfns, vb->num_pfns);
+	tell_host(vb, vb->deflate_vq, n);
+	release_pages_by_pfn(vb->pfns, n);
 }
 
 static inline void update_stat(struct virtio_balloon *vb, int idx,
-- 
1.7.9.5


  parent reply	other threads:[~2012-04-12  5:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-04-12  5:36 [PATCH 0/3] Bugfixes for virtio balloon driver David Gibson
2012-04-12  5:36 ` David Gibson
2012-04-12  5:36 ` [PATCH 1/3] virtio_balloon: Remove unnecessarily persistent state David Gibson
2012-04-12  5:36 ` David Gibson [this message]
2012-04-12  8:11   ` Michael S. Tsirkin
2012-04-12  8:11     ` Michael S. Tsirkin
2012-04-12  8:25   ` Michael S. Tsirkin
2012-04-12  8:25     ` Michael S. Tsirkin
2012-04-12  5:36 ` [PATCH 2/3] virtio_balloon: Fix endian bug David Gibson
2012-04-12  5:36 ` David Gibson
2012-04-12  8:30   ` Michael S. Tsirkin
2012-04-12  8:30     ` Michael S. Tsirkin
2012-04-12  5:36 ` [PATCH 3/3] virtio_balloon: Bugfixes for PAGE_SIZE != 4k David Gibson
2012-04-12  5:36 ` David Gibson
2012-04-12 10:14   ` Michael S. Tsirkin
2012-04-12 10:14     ` Michael S. Tsirkin
2012-04-13  3:12     ` David Gibson
2012-04-13  3:12       ` David Gibson
2012-04-15  8:52       ` Michael S. Tsirkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1334208995-29985-2-git-send-email-david@gibson.dropbear.id.au \
    --to=david@gibson.dropbear.id.au \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mst@redhat.com \
    --cc=paulus@samba.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rusty@rustcorp.com.au \
    --cc=virtualization@lists.linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.