Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1490109 > unrolled thread
| Started by | "Denis V. Lunev" <den@openvz.org> |
|---|---|
| First post | 2016-09-23 15:50 +0200 |
| Last post | 2016-09-28 16:10 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
[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
| From | "Denis V. Lunev" <den@openvz.org> |
|---|---|
| Date | 2016-09-23 15:50 +0200 |
| Subject | [PATCH 1/1] update balloon size in balloon "probe" |
| Message-ID | <skz1w-4hK-35@gated-at.bofh.it> |
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.
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:
--
2.7.4
[toc] | [next] | [standalone]
| From | "Michael S. Tsirkin" <mst@redhat.com> |
|---|---|
| Date | 2016-09-23 21:00 +0200 |
| Message-ID | <skDRv-7vd-7@gated-at.bofh.it> |
| In reply to | #1490109 |
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
> 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.
> --
> 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Konstantin Neumoin <kneumoin@virtuozzo.com> |
|---|---|
| Date | 2016-09-26 09:20 +0200 |
| Message-ID | <slymJ-QV-9@gated-at.bofh.it> |
| In reply to | #1490390 |
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.
>
>> --
>> 2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Konstantin Neumoin <kneumoin@virtuozzo.com> |
|---|---|
| Date | 2016-09-28 16:10 +0200 |
| Message-ID | <smnIB-7Ly-13@gated-at.bofh.it> |
| In reply to | #1491096 |
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
>
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web