Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2016-01-01 Thread Michael S. Tsirkin
On Fri, Jan 01, 2016 at 12:11:02PM +0200, Michael S. Tsirkin wrote:
> On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> > fill_balloon() and leak_balloon() manipulate only a limited number
> > of pages in one call. This is the reason why remove_common() calls
> > leak_balloon() in a while cycle.
> > 
> > remove_common() is called also when the system is being frozen.
> > But fill_balloon() is called only once when the system is being
> > restored. It means that most of the balloon stays leaked after
> > the system freeze and restore.
> 
> Right, but refilling might take a long while.
> In fact, we sleep for 200msec on refill failure,
> stalling system resume - which is already a bug.
> 
> > This patch adds the missing while cycle also into virtballoon_restore().
> > Also it makes fill_balloon() to return the number of really modified
> > pages. Note that leak_balloon() already did this.
> > 
> > Signed-off-by: Petr Mladek 
> 
> This is a replacement for:
>   virtio_balloon: Restore the entire balloon after the system freeze

oops, typo
I meant to write I'll send a replacement patch shortly :)

> > ---
> >  drivers/virtio/virtio_balloon.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c 
> > b/drivers/virtio/virtio_balloon.c
> > index 7efc32945810..d73a86db2490 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page 
> > *page)
> > pfns[i] = page_to_balloon_pfn(page) + i;
> >  }
> >  
> > -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> > +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> >  {
> > struct balloon_dev_info *vb_dev_info = >vb_dev_info;
> > +   unsigned num_allocated_pages;
> >  
> > /* We can only do one array worth at a time. */
> > num = min(num, ARRAY_SIZE(vb->pfns));
> > @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
> > size_t num)
> > adjust_managed_page_count(page, -1);
> > }
> >  
> > +   num_allocated_pages = vb->num_pfns;
> > /* Did we get any? */
> > if (vb->num_pfns != 0)
> > tell_host(vb, vb->inflate_vq);
> > mutex_unlock(>balloon_lock);
> > +
> > +   return num_allocated_pages;
> >  }
> >  
> >  static void release_pages_balloon(struct virtio_balloon *vb)
> > @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device 
> > *vdev)
> >  static int virtballoon_restore(struct virtio_device *vdev)
> >  {
> > struct virtio_balloon *vb = vdev->priv;
> > +   s64 diff;
> > int ret;
> >  
> > ret = init_vqs(vdev->priv);
> > @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device 
> > *vdev)
> >  
> > virtio_device_ready(vdev);
> >  
> > -   fill_balloon(vb, towards_target(vb));
> > +   diff = towards_target(vb);
> > +   while (diff > 0)
> > +   diff -= fill_balloon(vb, diff);
> > update_balloon_size(vb);
> > return 0;
> >  }
> > -- 
> > 1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2016-01-01 Thread Michael S. Tsirkin
On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> fill_balloon() and leak_balloon() manipulate only a limited number
> of pages in one call. This is the reason why remove_common() calls
> leak_balloon() in a while cycle.
> 
> remove_common() is called also when the system is being frozen.
> But fill_balloon() is called only once when the system is being
> restored. It means that most of the balloon stays leaked after
> the system freeze and restore.

Right, but refilling might take a long while.
In fact, we sleep for 200msec on refill failure,
stalling system resume - which is already a bug.

> This patch adds the missing while cycle also into virtballoon_restore().
> Also it makes fill_balloon() to return the number of really modified
> pages. Note that leak_balloon() already did this.
> 
> Signed-off-by: Petr Mladek 

This is a replacement for:
virtio_balloon: Restore the entire balloon after the system freeze

> ---
>  drivers/virtio/virtio_balloon.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc32945810..d73a86db2490 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
>   pfns[i] = page_to_balloon_pfn(page) + i;
>  }
>  
> -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>  {
>   struct balloon_dev_info *vb_dev_info = >vb_dev_info;
> + unsigned num_allocated_pages;
>  
>   /* We can only do one array worth at a time. */
>   num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
> size_t num)
>   adjust_managed_page_count(page, -1);
>   }
>  
> + num_allocated_pages = vb->num_pfns;
>   /* Did we get any? */
>   if (vb->num_pfns != 0)
>   tell_host(vb, vb->inflate_vq);
>   mutex_unlock(>balloon_lock);
> +
> + return num_allocated_pages;
>  }
>  
>  static void release_pages_balloon(struct virtio_balloon *vb)
> @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
>  static int virtballoon_restore(struct virtio_device *vdev)
>  {
>   struct virtio_balloon *vb = vdev->priv;
> + s64 diff;
>   int ret;
>  
>   ret = init_vqs(vdev->priv);
> @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
>  
>   virtio_device_ready(vdev);
>  
> - fill_balloon(vb, towards_target(vb));
> + diff = towards_target(vb);
> + while (diff > 0)
> + diff -= fill_balloon(vb, diff);
>   update_balloon_size(vb);
>   return 0;
>  }
> -- 
> 1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2016-01-01 Thread Michael S. Tsirkin
On Fri, Jan 01, 2016 at 12:11:02PM +0200, Michael S. Tsirkin wrote:
> On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> > fill_balloon() and leak_balloon() manipulate only a limited number
> > of pages in one call. This is the reason why remove_common() calls
> > leak_balloon() in a while cycle.
> > 
> > remove_common() is called also when the system is being frozen.
> > But fill_balloon() is called only once when the system is being
> > restored. It means that most of the balloon stays leaked after
> > the system freeze and restore.
> 
> Right, but refilling might take a long while.
> In fact, we sleep for 200msec on refill failure,
> stalling system resume - which is already a bug.
> 
> > This patch adds the missing while cycle also into virtballoon_restore().
> > Also it makes fill_balloon() to return the number of really modified
> > pages. Note that leak_balloon() already did this.
> > 
> > Signed-off-by: Petr Mladek 
> 
> This is a replacement for:
>   virtio_balloon: Restore the entire balloon after the system freeze

oops, typo
I meant to write I'll send a replacement patch shortly :)

> > ---
> >  drivers/virtio/virtio_balloon.c | 11 +--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c 
> > b/drivers/virtio/virtio_balloon.c
> > index 7efc32945810..d73a86db2490 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page 
> > *page)
> > pfns[i] = page_to_balloon_pfn(page) + i;
> >  }
> >  
> > -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> > +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
> >  {
> > struct balloon_dev_info *vb_dev_info = >vb_dev_info;
> > +   unsigned num_allocated_pages;
> >  
> > /* We can only do one array worth at a time. */
> > num = min(num, ARRAY_SIZE(vb->pfns));
> > @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
> > size_t num)
> > adjust_managed_page_count(page, -1);
> > }
> >  
> > +   num_allocated_pages = vb->num_pfns;
> > /* Did we get any? */
> > if (vb->num_pfns != 0)
> > tell_host(vb, vb->inflate_vq);
> > mutex_unlock(>balloon_lock);
> > +
> > +   return num_allocated_pages;
> >  }
> >  
> >  static void release_pages_balloon(struct virtio_balloon *vb)
> > @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device 
> > *vdev)
> >  static int virtballoon_restore(struct virtio_device *vdev)
> >  {
> > struct virtio_balloon *vb = vdev->priv;
> > +   s64 diff;
> > int ret;
> >  
> > ret = init_vqs(vdev->priv);
> > @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device 
> > *vdev)
> >  
> > virtio_device_ready(vdev);
> >  
> > -   fill_balloon(vb, towards_target(vb));
> > +   diff = towards_target(vb);
> > +   while (diff > 0)
> > +   diff -= fill_balloon(vb, diff);
> > update_balloon_size(vb);
> > return 0;
> >  }
> > -- 
> > 1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2016-01-01 Thread Michael S. Tsirkin
On Fri, Dec 04, 2015 at 02:37:50PM +0100, Petr Mladek wrote:
> fill_balloon() and leak_balloon() manipulate only a limited number
> of pages in one call. This is the reason why remove_common() calls
> leak_balloon() in a while cycle.
> 
> remove_common() is called also when the system is being frozen.
> But fill_balloon() is called only once when the system is being
> restored. It means that most of the balloon stays leaked after
> the system freeze and restore.

Right, but refilling might take a long while.
In fact, we sleep for 200msec on refill failure,
stalling system resume - which is already a bug.

> This patch adds the missing while cycle also into virtballoon_restore().
> Also it makes fill_balloon() to return the number of really modified
> pages. Note that leak_balloon() already did this.
> 
> Signed-off-by: Petr Mladek 

This is a replacement for:
virtio_balloon: Restore the entire balloon after the system freeze

> ---
>  drivers/virtio/virtio_balloon.c | 11 +--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc32945810..d73a86db2490 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
>   pfns[i] = page_to_balloon_pfn(page) + i;
>  }
>  
> -static void fill_balloon(struct virtio_balloon *vb, size_t num)
> +static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
>  {
>   struct balloon_dev_info *vb_dev_info = >vb_dev_info;
> + unsigned num_allocated_pages;
>  
>   /* We can only do one array worth at a time. */
>   num = min(num, ARRAY_SIZE(vb->pfns));
> @@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
> size_t num)
>   adjust_managed_page_count(page, -1);
>   }
>  
> + num_allocated_pages = vb->num_pfns;
>   /* Did we get any? */
>   if (vb->num_pfns != 0)
>   tell_host(vb, vb->inflate_vq);
>   mutex_unlock(>balloon_lock);
> +
> + return num_allocated_pages;
>  }
>  
>  static void release_pages_balloon(struct virtio_balloon *vb)
> @@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
>  static int virtballoon_restore(struct virtio_device *vdev)
>  {
>   struct virtio_balloon *vb = vdev->priv;
> + s64 diff;
>   int ret;
>  
>   ret = init_vqs(vdev->priv);
> @@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
>  
>   virtio_device_ready(vdev);
>  
> - fill_balloon(vb, towards_target(vb));
> + diff = towards_target(vb);
> + while (diff > 0)
> + diff -= fill_balloon(vb, diff);
>   update_balloon_size(vb);
>   return 0;
>  }
> -- 
> 1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2015-12-04 Thread Petr Mladek
fill_balloon() and leak_balloon() manipulate only a limited number
of pages in one call. This is the reason why remove_common() calls
leak_balloon() in a while cycle.

remove_common() is called also when the system is being frozen.
But fill_balloon() is called only once when the system is being
restored. It means that most of the balloon stays leaked after
the system freeze and restore.

This patch adds the missing while cycle also into virtballoon_restore().
Also it makes fill_balloon() to return the number of really modified
pages. Note that leak_balloon() already did this.

Signed-off-by: Petr Mladek 
---
 drivers/virtio/virtio_balloon.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7efc32945810..d73a86db2490 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
pfns[i] = page_to_balloon_pfn(page) + i;
 }
 
-static void fill_balloon(struct virtio_balloon *vb, size_t num)
+static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 {
struct balloon_dev_info *vb_dev_info = >vb_dev_info;
+   unsigned num_allocated_pages;
 
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
size_t num)
adjust_managed_page_count(page, -1);
}
 
+   num_allocated_pages = vb->num_pfns;
/* Did we get any? */
if (vb->num_pfns != 0)
tell_host(vb, vb->inflate_vq);
mutex_unlock(>balloon_lock);
+
+   return num_allocated_pages;
 }
 
 static void release_pages_balloon(struct virtio_balloon *vb)
@@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 static int virtballoon_restore(struct virtio_device *vdev)
 {
struct virtio_balloon *vb = vdev->priv;
+   s64 diff;
int ret;
 
ret = init_vqs(vdev->priv);
@@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
 
virtio_device_ready(vdev);
 
-   fill_balloon(vb, towards_target(vb));
+   diff = towards_target(vb);
+   while (diff > 0)
+   diff -= fill_balloon(vb, diff);
update_balloon_size(vb);
return 0;
 }
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH v4 1/2] virtio_balloon: Restore the entire balloon after the system freeze

2015-12-04 Thread Petr Mladek
fill_balloon() and leak_balloon() manipulate only a limited number
of pages in one call. This is the reason why remove_common() calls
leak_balloon() in a while cycle.

remove_common() is called also when the system is being frozen.
But fill_balloon() is called only once when the system is being
restored. It means that most of the balloon stays leaked after
the system freeze and restore.

This patch adds the missing while cycle also into virtballoon_restore().
Also it makes fill_balloon() to return the number of really modified
pages. Note that leak_balloon() already did this.

Signed-off-by: Petr Mladek 
---
 drivers/virtio/virtio_balloon.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7efc32945810..d73a86db2490 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -135,9 +135,10 @@ static void set_page_pfns(u32 pfns[], struct page *page)
pfns[i] = page_to_balloon_pfn(page) + i;
 }
 
-static void fill_balloon(struct virtio_balloon *vb, size_t num)
+static unsigned fill_balloon(struct virtio_balloon *vb, size_t num)
 {
struct balloon_dev_info *vb_dev_info = >vb_dev_info;
+   unsigned num_allocated_pages;
 
/* We can only do one array worth at a time. */
num = min(num, ARRAY_SIZE(vb->pfns));
@@ -162,10 +163,13 @@ static void fill_balloon(struct virtio_balloon *vb, 
size_t num)
adjust_managed_page_count(page, -1);
}
 
+   num_allocated_pages = vb->num_pfns;
/* Did we get any? */
if (vb->num_pfns != 0)
tell_host(vb, vb->inflate_vq);
mutex_unlock(>balloon_lock);
+
+   return num_allocated_pages;
 }
 
 static void release_pages_balloon(struct virtio_balloon *vb)
@@ -581,6 +585,7 @@ static int virtballoon_freeze(struct virtio_device *vdev)
 static int virtballoon_restore(struct virtio_device *vdev)
 {
struct virtio_balloon *vb = vdev->priv;
+   s64 diff;
int ret;
 
ret = init_vqs(vdev->priv);
@@ -589,7 +594,9 @@ static int virtballoon_restore(struct virtio_device *vdev)
 
virtio_device_ready(vdev);
 
-   fill_balloon(vb, towards_target(vb));
+   diff = towards_target(vb);
+   while (diff > 0)
+   diff -= fill_balloon(vb, diff);
update_balloon_size(vb);
return 0;
 }
-- 
1.8.5.6

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/