From patchwork Wed Mar 4 06:14:54 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rusty Russell X-Patchwork-Id: 5933371 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0EEE29F318 for ; Wed, 4 Mar 2015 08:14:45 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2FDCF20160 for ; Wed, 4 Mar 2015 08:14:44 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AEB842013A for ; Wed, 4 Mar 2015 08:14:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759997AbbCDIOX (ORCPT ); Wed, 4 Mar 2015 03:14:23 -0500 Received: from ozlabs.org ([103.22.144.67]:36826 "EHLO ozlabs.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758799AbbCDGPe (ORCPT ); Wed, 4 Mar 2015 01:15:34 -0500 Received: by ozlabs.org (Postfix, from userid 1011) id 2788C1401E7; Wed, 4 Mar 2015 17:15:33 +1100 (AEDT) From: Rusty Russell To: "Michael S. Tsirkin" Cc: Thomas Huth , virtualization@lists.linux-foundation.org, kvm@vger.kernel.org, Peter Zijlstra Subject: Re: virtio balloon: do not call blocking ops when !TASK_RUNNING In-Reply-To: <20150302111358.GA4954@redhat.com> References: <20150225111318.0040536b@oc7435384737.ibm.com> <87y4nllb85.fsf@rustcorp.com.au> <20150226083625.2520ecb6@oc7435384737.ibm.com> <8761akl0sh.fsf@rustcorp.com.au> <20150302111358.GA4954@redhat.com> User-Agent: Notmuch/0.17 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-pc-linux-gnu) Date: Wed, 04 Mar 2015 16:44:54 +1030 Message-ID: <87d24pjnkx.fsf@rustcorp.com.au> MIME-Version: 1.0 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, T_RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP "Michael S. Tsirkin" writes: > On Mon, Mar 02, 2015 at 10:37:26AM +1030, Rusty Russell wrote: >> Thomas Huth writes: >> > On Thu, 26 Feb 2015 11:50:42 +1030 >> > Rusty Russell wrote: >> > >> >> Thomas Huth writes: >> >> > Hi all, >> >> > >> >> > with the recent kernel 3.19, I get a kernel warning when I start my >> >> > KVM guest on s390 with virtio balloon enabled: >> >> >> >> The deeper problem is that virtio_ccw_get_config just silently fails on >> >> OOM. >> >> >> >> Neither get_config nor set_config are expected to fail. >> > >> > AFAIK this is currently not a problem. According to >> > http://lwn.net/Articles/627419/ these kmalloc calls never >> > fail because they allocate less than a page. >> >> I strongly suggest you unlearn that fact. >> The fix for this is in two parts: >> >> 1) Annotate using sched_annotate_sleep() and add a comment: we may spin >> a few times in low memory situations, but this isn't a high >> performance path. >> >> 2) Handle get_config (and other) failure in some more elegant way. >> >> Cheers, >> Rusty. > > I agree, but I'd like to point out that even without kmalloc, > on s390 get_config is blocking - it's waiting > for a hardware interrupt. > > And it makes sense: config is not data path, I don't think > we should spin there. > > So I think besides these two parts, we still need my two patches: > virtio-balloon: do not call blocking ops when !TASK_RUNNING I prefer to annotate, over trying to fix this. Because it's not important. We might spin a few times, but it's very unlikely, and it's certainly not performance critical. Thanks, Rusty. Subject: virtio_balloon: annotate possible sleep waiting for event. CCW (s390) does this. Reported-by: Thomas Huth Signed-off-by: Rusty Russell --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/virtio/virtio_balloon.c b/drivers/virtio/virtio_balloon.c index 0413157f3b49..3f4d5acdbde0 100644 --- a/drivers/virtio/virtio_balloon.c +++ b/drivers/virtio/virtio_balloon.c @@ -340,6 +340,15 @@ static int balloon(void *_vballoon) s64 diff; try_to_freeze(); + + /* + * Reading the config on the ccw backend involves an + * allocation, so we may actually sleep and have an + * extra iteration. It's extremely unlikely, and this + * isn't a fast path in any sense. + */ + sched_annotate_sleep(); + wait_event_interruptible(vb->config_change, (diff = towards_target(vb)) != 0 || vb->need_stats_update