From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932589AbcI1OEQ (ORCPT ); Wed, 28 Sep 2016 10:04:16 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:16290 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932209AbcI1OEK (ORCPT ); Wed, 28 Sep 2016 10:04:10 -0400 Reply-To: kneumoin@virtuozzo.com Subject: Re: [PATCH 1/1] update balloon size in balloon "probe" References: <1474638477-8658-1-git-send-email-den@openvz.org> <20160923215334-mutt-send-email-mst@kernel.org> To: "Michael S. Tsirkin" , "Denis V. Lunev" Cc: virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org From: Konstantin Neumoin Organization: Virtuozzo Message-ID: <4f17023e-7e29-2c55-f7b7-0eff2dd014b3@virtuozzo.com> Date: Wed, 28 Sep 2016 17:03:50 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 >>> >>> Patch >>> Commit 3d2a3774c1b046f548ebea0391a602fd5685a307 >>> Author: Michael S. Tsirkin >>> 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 >> 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 >>> Signed-off-by: Denis V. Lunev >>> CC: "Michael S. Tsirkin" >>> --- >>> 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 > >