All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC] virtio_balloon: refill by config handler
@ 2016-01-01 10:13 ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-01-01 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Petr Mladek, virtualization, Jeff Epler, Tejun Heo, Jiri Kosina

Peter Mladek reported that balloon might never refill completely after
restore.  This is because fill_balloon is only called once there.
Calling fill_balloon repeatedly seems too aggressive,
especially in light of the fact that it sleeps on failure: let's
wake the config change handler and fill it asynchronously.

Reported-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I was unable to test this - for some reason my test VM
doesn't resume (with or without the patch).
Petr, does this work for you?

 drivers/virtio/virtio_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7efc329..ee29473 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
-	fill_balloon(vb, towards_target(vb));
+	wake_up(&vb->config_change);
 	update_balloon_size(vb);
 	return 0;
 }
-- 
MST

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH RFC] virtio_balloon: refill by config handler
@ 2016-01-01 10:13 ` Michael S. Tsirkin
  0 siblings, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-01-01 10:13 UTC (permalink / raw)
  To: linux-kernel
  Cc: Tejun Heo, Petr Mladek, Jeff Epler, Jiri Kosina, virtualization

Peter Mladek reported that balloon might never refill completely after
restore.  This is because fill_balloon is only called once there.
Calling fill_balloon repeatedly seems too aggressive,
especially in light of the fact that it sleeps on failure: let's
wake the config change handler and fill it asynchronously.

Reported-by: Petr Mladek <pmladek@suse.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

I was unable to test this - for some reason my test VM
doesn't resume (with or without the patch).
Petr, does this work for you?

 drivers/virtio/virtio_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
index 7efc329..ee29473 100644
--- a/drivers/virtio/virtio_balloon.c
+++ b/drivers/virtio/virtio_balloon.c
@@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
 
 	virtio_device_ready(vdev);
 
-	fill_balloon(vb, towards_target(vb));
+	wake_up(&vb->config_change);
 	update_balloon_size(vb);
 	return 0;
 }
-- 
MST

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH RFC] virtio_balloon: refill by config handler
  2016-01-01 10:13 ` Michael S. Tsirkin
  (?)
@ 2016-01-05 14:37 ` Petr Mladek
  2016-01-05 15:31   ` Michael S. Tsirkin
  2016-01-05 15:31   ` Michael S. Tsirkin
  -1 siblings, 2 replies; 6+ messages in thread
From: Petr Mladek @ 2016-01-05 14:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel, virtualization, Jeff Epler, Tejun Heo, Jiri Kosina

On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> Peter Mladek reported that balloon might never refill completely after
> restore.  This is because fill_balloon is only called once there.
> Calling fill_balloon repeatedly seems too aggressive,
> especially in light of the fact that it sleeps on failure: let's
> wake the config change handler and fill it asynchronously.
> 
> Reported-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> I was unable to test this - for some reason my test VM
> doesn't resume (with or without the patch).
> Petr, does this work for you?

Ah, I have just realized that the problem happens only after the
conversion to the workqueue. It works without this patch with the
current code. The balloon kthread is waken when released from
__refrigerator().

I am sorry for the noise. I wanted to split the conversion to
workqueues into two patches to make it better readable. We need to
queue the balloon resizing work here. I wrongly assumed that the
original kthread must have the same problem.

Best Regards,
Petr

>  drivers/virtio/virtio_balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc329..ee29473 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
>  
>  	virtio_device_ready(vdev);
>  
> -	fill_balloon(vb, towards_target(vb));
> +	wake_up(&vb->config_change);
>  	update_balloon_size(vb);
>  	return 0;
>  }
> -- 
> MST

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RFC] virtio_balloon: refill by config handler
  2016-01-01 10:13 ` Michael S. Tsirkin
  (?)
  (?)
@ 2016-01-05 14:37 ` Petr Mladek
  -1 siblings, 0 replies; 6+ messages in thread
From: Petr Mladek @ 2016-01-05 14:37 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Tejun Heo, Jiri Kosina, Jeff Epler, linux-kernel, virtualization

On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> Peter Mladek reported that balloon might never refill completely after
> restore.  This is because fill_balloon is only called once there.
> Calling fill_balloon repeatedly seems too aggressive,
> especially in light of the fact that it sleeps on failure: let's
> wake the config change handler and fill it asynchronously.
> 
> Reported-by: Petr Mladek <pmladek@suse.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> I was unable to test this - for some reason my test VM
> doesn't resume (with or without the patch).
> Petr, does this work for you?

Ah, I have just realized that the problem happens only after the
conversion to the workqueue. It works without this patch with the
current code. The balloon kthread is waken when released from
__refrigerator().

I am sorry for the noise. I wanted to split the conversion to
workqueues into two patches to make it better readable. We need to
queue the balloon resizing work here. I wrongly assumed that the
original kthread must have the same problem.

Best Regards,
Petr

>  drivers/virtio/virtio_balloon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 7efc329..ee29473 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
>  
>  	virtio_device_ready(vdev);
>  
> -	fill_balloon(vb, towards_target(vb));
> +	wake_up(&vb->config_change);
>  	update_balloon_size(vb);
>  	return 0;
>  }
> -- 
> MST

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RFC] virtio_balloon: refill by config handler
  2016-01-05 14:37 ` Petr Mladek
@ 2016-01-05 15:31   ` Michael S. Tsirkin
  2016-01-05 15:31   ` Michael S. Tsirkin
  1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-01-05 15:31 UTC (permalink / raw)
  To: Petr Mladek
  Cc: linux-kernel, virtualization, Jeff Epler, Tejun Heo, Jiri Kosina

On Tue, Jan 05, 2016 at 03:37:33PM +0100, Petr Mladek wrote:
> On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> > Peter Mladek reported that balloon might never refill completely after
> > restore.  This is because fill_balloon is only called once there.
> > Calling fill_balloon repeatedly seems too aggressive,
> > especially in light of the fact that it sleeps on failure: let's
> > wake the config change handler and fill it asynchronously.
> > 
> > Reported-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> > I was unable to test this - for some reason my test VM
> > doesn't resume (with or without the patch).
> > Petr, does this work for you?
> 
> Ah, I have just realized that the problem happens only after the
> conversion to the workqueue. It works without this patch with the
> current code. The balloon kthread is waken when released from
> __refrigerator().

In fact, maybe we should drop fill_balloon and
update_balloon_size here?
Resume will finish faster ...

> I am sorry for the noise. I wanted to split the conversion to
> workqueues into two patches to make it better readable. We need to
> queue the balloon resizing work here. I wrongly assumed that the
> original kthread must have the same problem.
> 
> Best Regards,
> Petr
> 
> >  drivers/virtio/virtio_balloon.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 7efc329..ee29473 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
> >  
> >  	virtio_device_ready(vdev);
> >  
> > -	fill_balloon(vb, towards_target(vb));
> > +	wake_up(&vb->config_change);
> >  	update_balloon_size(vb);
> >  	return 0;
> >  }
> > -- 
> > MST

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH RFC] virtio_balloon: refill by config handler
  2016-01-05 14:37 ` Petr Mladek
  2016-01-05 15:31   ` Michael S. Tsirkin
@ 2016-01-05 15:31   ` Michael S. Tsirkin
  1 sibling, 0 replies; 6+ messages in thread
From: Michael S. Tsirkin @ 2016-01-05 15:31 UTC (permalink / raw)
  To: Petr Mladek
  Cc: Tejun Heo, Jiri Kosina, Jeff Epler, linux-kernel, virtualization

On Tue, Jan 05, 2016 at 03:37:33PM +0100, Petr Mladek wrote:
> On Fri 2016-01-01 12:13:48, Michael S. Tsirkin wrote:
> > Peter Mladek reported that balloon might never refill completely after
> > restore.  This is because fill_balloon is only called once there.
> > Calling fill_balloon repeatedly seems too aggressive,
> > especially in light of the fact that it sleeps on failure: let's
> > wake the config change handler and fill it asynchronously.
> > 
> > Reported-by: Petr Mladek <pmladek@suse.com>
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> > 
> > I was unable to test this - for some reason my test VM
> > doesn't resume (with or without the patch).
> > Petr, does this work for you?
> 
> Ah, I have just realized that the problem happens only after the
> conversion to the workqueue. It works without this patch with the
> current code. The balloon kthread is waken when released from
> __refrigerator().

In fact, maybe we should drop fill_balloon and
update_balloon_size here?
Resume will finish faster ...

> I am sorry for the noise. I wanted to split the conversion to
> workqueues into two patches to make it better readable. We need to
> queue the balloon resizing work here. I wrongly assumed that the
> original kthread must have the same problem.
> 
> Best Regards,
> Petr
> 
> >  drivers/virtio/virtio_balloon.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> > index 7efc329..ee29473 100644
> > --- a/drivers/virtio/virtio_balloon.c
> > +++ b/drivers/virtio/virtio_balloon.c
> > @@ -589,7 +589,7 @@ static int virtballoon_restore(struct virtio_device *vdev)
> >  
> >  	virtio_device_ready(vdev);
> >  
> > -	fill_balloon(vb, towards_target(vb));
> > +	wake_up(&vb->config_change);
> >  	update_balloon_size(vb);
> >  	return 0;
> >  }
> > -- 
> > MST

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-05 15:31 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-01 10:13 [PATCH RFC] virtio_balloon: refill by config handler Michael S. Tsirkin
2016-01-01 10:13 ` Michael S. Tsirkin
2016-01-05 14:37 ` Petr Mladek
2016-01-05 15:31   ` Michael S. Tsirkin
2016-01-05 15:31   ` Michael S. Tsirkin
2016-01-05 14:37 ` Petr Mladek

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.