Re: [PATCH 1/1] update balloon size in balloon "probe"

[Michael S. Tsirkin]

On Fri, Sep 23, 2016 at 04:47:57PM +0300, Denis V. Lunev wrote:
> From: Konstantin Neumoin <kneumoin@xxxxxxxxxxxxx>
> 
> Patch
>     Commit 3d2a3774c1b046f548ebea0391a602fd5685a307
>     Author: Michael S. Tsirkin <mst@xxxxxxxxxx>
>     Date:   Tue Mar 10 11:55:08 2015 +1030
>     virtio-balloon: do not call blocking ops when !TASK_RUNNING
> has added a regression. Original code with wait_event_interruptible
> checked the condition before start waiting and started balloon operations
> if necessary.

I don't get it, sorry.

+               add_wait_queue(&vb->config_change, &wait);
+               for (;;) {
+                       if ((diff = towards_target(vb)) != 0 ||
+                           vb->need_stats_update ||
+                           kthread_should_stop() ||
+                           freezing(current))
+                               break;
+                       wait_woken(&wait, TASK_INTERRUPTIBLE, MAX_SCHEDULE_TIMEOUT);
+               }
+               remove_wait_queue(&vb->config_change, &wait);

Seems to check the condition before waiting.

The issue is more likely with this patch:

	commit fad7b7b27b6a168ca8ebc84482043886f837b89d
	Author: Petr Mladek <pmladek@xxxxxxxx>
	Date:   Mon Jan 25 17:38:05 2016 +0100

	    virtio_balloon: Use a workqueue instead of "vballoon" kthread
    


> Right now balloon is not inflated if ballon target is set before the
> driver is loaded.
> 
> Signed-off-by: Konstantin Neumoin <kneumoin@xxxxxxxxxxxxx>
> Signed-off-by: Denis V. Lunev <den@xxxxxxxxxx>
> CC: "Michael S. Tsirkin" <mst@xxxxxxxxxx>
> ---
>  drivers/virtio/virtio_balloon.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c
> index 4e7003d..0a6c10f 100644
> --- a/drivers/virtio/virtio_balloon.c
> +++ b/drivers/virtio/virtio_balloon.c
> @@ -577,6 +577,10 @@ static int virtballoon_probe(struct virtio_device *vdev)
>  
>  	virtio_device_ready(vdev);
>  
> +	if (towards_target(vb))
> +		virtballoon_changed(vdev);
> +	update_balloon_size(vb);
> +
>  	return 0;
>  
>  out_del_vqs:


I know we have same thing on restore, but it seems bogus
there as well:
        if (towards_target(vb))
                virtballoon_changed(vdev);
        update_balloon_size(vb);

makes no sense because virtballoon_changed merely queues
the work.


> -- 
> 2.7.4