Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1492618
| From | Konstantin Neumoin <kneumoin@virtuozzo.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 1/1] update balloon size in balloon "probe" |
| Date | 2016-09-28 16:10 +0200 |
| Message-ID | <smnIB-7Ly-13@gated-at.bofh.it> (permalink) |
| References | <skz1w-4hK-35@gated-at.bofh.it> <skDRv-7vd-7@gated-at.bofh.it> <slymJ-QV-9@gated-at.bofh.it> |
| Organization | Virtuozzo |
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@virtuozzo.com>
>>>
>>> Patch
>>> Commit 3d2a3774c1b046f548ebea0391a602fd5685a307
>>> Author: Michael S. Tsirkin <mst@redhat.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@suse.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@virtuozzo.com>
>>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>>> CC: "Michael S. Tsirkin" <mst@redhat.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
>
>
Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread
[PATCH 1/1] update balloon size in balloon "probe" "Denis V. Lunev" <den@openvz.org> - 2016-09-23 15:50 +0200
Re: [PATCH 1/1] update balloon size in balloon "probe" "Michael S. Tsirkin" <mst@redhat.com> - 2016-09-23 21:00 +0200
Re: [PATCH 1/1] update balloon size in balloon "probe" Konstantin Neumoin <kneumoin@virtuozzo.com> - 2016-09-26 09:20 +0200
Re: [PATCH 1/1] update balloon size in balloon "probe" Konstantin Neumoin <kneumoin@virtuozzo.com> - 2016-09-28 16:10 +0200
csiph-web