On Sunday 09 March 2008 06:06:38 Marcelo Tosatti wrote: > But making the driver robust against it seems sensate. I agree that > zeroing num_pages is hackish. What do you suggest?
OK, after more discussion on IRC, this seems like the correct thing to do. 1) Allow more than 2G of pages. It doesn't cost us much to break the 8TB barrier. This is an ABI change, so best to do this now. 2) Handle the case where we get nonsense from the host, which causes us to wrap around. How's this (compile-tested only!): diff -r fd0c80dbbd95 drivers/virtio/virtio_balloon.c --- a/drivers/virtio/virtio_balloon.c Tue Mar 11 09:21:00 2008 +1100 +++ b/drivers/virtio/virtio_balloon.c Tue Mar 11 11:25:52 2008 +1100 @@ -43,12 +43,12 @@ struct virtio_balloon bool tell_host_first; /* The pages we've told the Host we're not using. */ - unsigned int num_pages; + unsigned long num_pages; struct list_head pages; /* The array of pfns we tell the Host about. */ unsigned int num_pfns; - u32 pfns[256]; + u64 pfns[256]; }; static struct virtio_device_id id_table[] = { @@ -83,17 +83,17 @@ static void tell_host(struct virtio_ball wait_for_completion(&vb->acked); } -static void fill_balloon(struct virtio_balloon *vb, size_t num) +static void fill_balloon(struct virtio_balloon *vb, u64 num) { /* We can only do one array worth at a time. */ - num = min(num, ARRAY_SIZE(vb->pfns)); + num = min_t(u64, num, ARRAY_SIZE(vb->pfns)); for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) { struct page *page = alloc_page(GFP_HIGHUSER | __GFP_NORETRY); if (!page) { if (printk_ratelimit()) dev_printk(KERN_INFO, &vb->vdev->dev, - "Out of puff! Can't get %zu pages\n", + "Out of puff! Can't get %llu pages\n", num); /* Sleep for at least 1/5 of a second before retry. */ msleep(200); @@ -112,7 +112,7 @@ static void fill_balloon(struct virtio_b tell_host(vb, vb->inflate_vq); } -static void release_pages_by_pfn(const u32 pfns[], unsigned int num) +static void release_pages_by_pfn(const u64 pfns[], unsigned int num) { unsigned int i; @@ -122,12 +122,12 @@ static void release_pages_by_pfn(const u } } -static void leak_balloon(struct virtio_balloon *vb, size_t num) +static void leak_balloon(struct virtio_balloon *vb, u64 num) { struct page *page; /* We can only do one array worth at a time. */ - num = min(num, ARRAY_SIZE(vb->pfns)); + num = min_t(u64, num, ARRAY_SIZE(vb->pfns)); for (vb->num_pfns = 0; vb->num_pfns < num; vb->num_pfns++) { page = list_first_entry(&vb->pages, struct page, lru); @@ -152,12 +152,19 @@ static void virtballoon_changed(struct v wake_up(&vb->config_change); } -static inline int towards_target(struct virtio_balloon *vb) +static inline s64 towards_target(struct virtio_balloon *vb) { - u32 v; + u64 v; __virtio_config_val(vb->vdev, offsetof(struct virtio_balloon_config, num_pages), &v); + + /* If they ask for something which will wrap, we ignore it. */ + if (v & 0x8000000000000000ULL) { + dev_warn(&vb->vdev->dev, "Insane target 0x%llx\n", v); + return 0; + } + return v - vb->num_pages; } @@ -176,7 +183,7 @@ static int balloon(void *_vballoon) set_freezable(); while (!kthread_should_stop()) { - int diff; + s64 diff; try_to_freeze(); wait_event_interruptible(vb->config_change, diff -r fd0c80dbbd95 include/linux/virtio_balloon.h --- a/include/linux/virtio_balloon.h Tue Mar 11 09:21:00 2008 +1100 +++ b/include/linux/virtio_balloon.h Tue Mar 11 11:25:52 2008 +1100 @@ -11,8 +11,8 @@ struct virtio_balloon_config struct virtio_balloon_config { /* Number of pages host wants Guest to give up. */ - __le32 num_pages; + __le64 num_pages; /* Number of pages we've actually got in balloon. */ - __le32 actual; + __le64 actual; }; #endif /* _LINUX_VIRTIO_BALLOON_H */ ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ kvm-devel mailing list kvm-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/kvm-devel