Re: [PATCH 3/3] virtio_balloon: Bugfixes for PAGE_SIZE != 4k

From: David Gibson
Date: Thu Apr 12 2012 - 23:12:44 EST


On Thu, Apr 12, 2012 at 01:14:06PM +0300, Michael S. Tsirkin wrote:
> On Thu, Apr 12, 2012 at 03:36:35PM +1000, David Gibson wrote:
[snip]
> Good catch!
>
> Unfortunately I find the approach a bit convoluted.
> It also looks like when host asks for 5 balloon pages
> you interpret this as 0 where 16 is probably saner
> on a 64K system.

Hm, true. Although qemu at least actuall operates in units of
megabytes on the balloon, so I doubt it matters much in practice.

> I think it's easier if we just keep doing math in
> balloon pages. I also get confused by shift operations,
> IMO / and * are clearer where they are applicable.
> Something like the below would make more sense I think.

Sure. I thught working in local pages was clearer, but I don't really
care.


> I also wrote up a detailed commit log, so we have
> the bugs and the expected consequences listed explicitly.
>
> I'm out of time for this week - so completely untested, sorry.
> Maybe you could try this out? That would be great.
> Thanks!

Your patch has numerous syntax errors, but once those are fixed seems
to work fine with a 64k ppc64 kernel. Fixed version below. I did add
one comment, to note that with this change the num_pages field in the
vb is no longer the same as the number of elements in the pages list.
Nothing in the code relies on that, but it would probably be the first
assumption of someone looking at the structure definition.

Please apply.