linux-kernel - Re: [PATCH v8 1/5] mm: introduce a common interface for balloon pages mobility

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 24 Aug 2012 02:36:16 +0300
From:	"Michael S. Tsirkin" <mst@...hat.com>
To:	Rafael Aquini <aquini@...hat.com>
Cc:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Peter Zijlstra <peterz@...radead.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	virtualization@...ts.linux-foundation.org,
	Rusty Russell <rusty@...tcorp.com.au>,
	Rik van Riel <riel@...hat.com>, Mel Gorman <mel@....ul.ie>,
	Andi Kleen <andi@...stfloor.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Minchan Kim <minchan@...nel.org>
Subject: Re: [PATCH v8 1/5] mm: introduce a common interface for balloon
 pages mobility

On Thu, Aug 23, 2012 at 02:28:45PM -0300, Rafael Aquini wrote:
> On Thu, Aug 23, 2012 at 07:25:05PM +0300, Michael S. Tsirkin wrote:
> > On Thu, Aug 23, 2012 at 04:53:28PM +0300, Michael S. Tsirkin wrote:
> > > Basically it was very simple: we assumed page->lru was never
> > > touched for an allocated page, so it's safe to use it for
> > > internal book-keeping by the driver.
> > > 
> > > Now, this is not the case anymore, you add some logic in mm/ that might
> > > or might not touch page->lru depending on things like reference count.
> > 
> > Another thought: would the issue go away if balloon used
> > page->private to link pages instead of LRU?
> > mm core could keep a reference on page to avoid it
> > being used while mm handles it (maybe it does already?).
> >
> I don't think so. That would be a lot more trikier and complex, IMHO.

What's tricky? Linking pages through a void * orivate pointer?
I can code it up in a couple of minutes.
It's middle of the night so too tired to test but still:

> > If we do this, will not the only change to balloon be to tell mm that it
> > can use compaction for these pages when it allocates the page: using
> > some GPF flag or a new API?
> > 
> 
> What about keep a conter at virtio_balloon structure on how much pages are
> isolated from balloon's list and check it at leak time?
> if the counter gets > 0 than we can safely put leak_balloon() to wait until
> balloon page list gets completely refilled. I guess that is simple to get
> accomplished and potentially addresses all your concerns on this issue.
> 
> Cheers!

I would wake it each time after adding a page, then it
can stop waiting when it leaks enough.
But again, it's cleaner to just keep tracking all
pages, let mm hang on to them by keeping a reference.

--->

virtio-balloon: replace page->lru list with page->private.

The point is to free up page->lru for use by compaction.
Warning: completely untested, will provide tested version
if we agree on this direction.

Signed-off-by: Michael S. Tsirkin <mst@...hat.com>

---

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 0908e60..b38f57ce 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -56,7 +56,7 @@ struct virtio_balloon
 	 * Each page on this list adds VIRTIO_BALLOON_PAGES_PER_PAGE
 	 * to num_pages above.
 	 */
-	struct list_head pages;
+	void *pages;
 
 	/* The array of pfns we tell the Host about. */
 	unsigned int num_pfns;
@@ -141,7 +141,9 @@ static void fill_balloon(struct virtio_balloon *vb, size_t num)
 		set_page_pfns(vb->pfns + vb->num_pfns, page);
 		vb->num_pages += VIRTIO_BALLOON_PAGES_PER_PAGE;
 		totalram_pages--;
-		list_add(&page->lru, &vb->pages);
+		/* Add to list of pages */
+		page->private = vb->pages;
+		vb->pages = page->private;
 	}
 
 	/* Didn't get any?  Oh well. */
@@ -171,8 +173,9 @@ static void leak_balloon(struct virtio_balloon *vb, size_t num)
 
 	for (vb->num_pfns = 0; vb->num_pfns < num;
 	     vb->num_pfns += VIRTIO_BALLOON_PAGES_PER_PAGE) {
-		page = list_first_entry(&vb->pages, struct page, lru);
-		list_del(&page->lru);
+		/* Delete from list of pages */
+		page = vb->pages;
+		vb->pages = page->private;
 		set_page_pfns(vb->pfns + vb->num_pfns, page);
 		vb->num_pages -= VIRTIO_BALLOON_PAGES_PER_PAGE;
 	}
@@ -350,7 +353,7 @@ static int virtballoon_probe(struct virtio_device *vdev)
 		goto out;
 	}
 
-	INIT_LIST_HEAD(&vb->pages);
+	vb->pages = NULL;
 	vb->num_pages = 0;
 	init_waitqueue_head(&vb->config_change);
 	init_waitqueue_head(&vb->acked);
-- 
MST
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/