From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761830AbcIZHQe (ORCPT ); Mon, 26 Sep 2016 03:16:34 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:20794 "EHLO relay.sw.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1761095AbcIZHQb (ORCPT ); Mon, 26 Sep 2016 03:16:31 -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: Date: Mon, 26 Sep 2016 10:16:14 +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: <20160923215334-mutt-send-email-mst@kernel.org> 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/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. > >> -- >> 2.7.4