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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4f17023e-7e29-2c55-f7b7-0eff2dd014b3@virtuozzo.com>
Date:   Wed, 28 Sep 2016 17:03:50 +0300
From:   Konstantin Neumoin <kneumoin@...tuozzo.com>
To:     "Michael S. Tsirkin" <mst@...hat.com>,
        "Denis V. Lunev" <den@...nvz.org>
Cc:     virtualization@...ts.linux-foundation.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] update balloon size in balloon "probe"

On 09/26/2016 10:16 AM, Konstantin Neumoin wrote:
> On 09/23/2016 09:58 PM, Michael S. Tsirkin wrote:
>> On Fri, Sep 23, 2016 at 04:47:57PM +0300, Denis V. Lunev wrote:
>>> From: Konstantin Neumoin <kneumoin@...tuozzo.com>
>>>
>>> Patch
>>>      Commit 3d2a3774c1b046f548ebea0391a602fd5685a307
>>>      Author: Michael S. Tsirkin <mst@...hat.com>
>>>      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@...e.com>
>>     Date:   Mon Jan 25 17:38:05 2016 +0100
>>
>>         virtio_balloon: Use a workqueue instead of "vballoon" kthread
> yes, you are right, sorry, commit message is incorrect.
>>
>>
>>> Right now balloon is not inflated if ballon target is set before the
>>> driver is loaded.
>>>
>>> Signed-off-by: Konstantin Neumoin <kneumoin@...tuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@...nvz.org>
>>> CC: "Michael S. Tsirkin" <mst@...hat.com>
>>> ---
>>>   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.
>  in virtballoon_probe we just init work items, but if we have a target 
> - num_pages != 0
> we will catch it only after virtballoon_changed will be called, isn't it?
>
>  So, I believe, on probe, we have to do the same thing as on restore: 
> check towards_target(vb)
> and call virtballoon_changed(vdev) if necessary.
>>

  Also we could try inflate the balloon once(by fill_balloon()) on probe 
and it leads to config change,
but we will not receive this notification because of config change 
pending will be enabled after
drv->prob() in virtio_dev_probe.
>>> -- 
>>> 2.7.4
>
>