From patchwork Tue Jun 20 13:00:32 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dario Faggioli X-Patchwork-Id: 9799493 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8D1F960329 for ; Tue, 20 Jun 2017 13:04:03 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8362522B1F for ; Tue, 20 Jun 2017 13:04:03 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 782FA2847B; Tue, 20 Jun 2017 13:04:03 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D33DF22B1F for ; Tue, 20 Jun 2017 13:04:01 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dNIlt-0002jf-T8; Tue, 20 Jun 2017 13:00:41 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1dNIls-0002jZ-Tr for xen-devel@lists.xenproject.org; Tue, 20 Jun 2017 13:00:41 +0000 Received: from [85.158.139.211] by server-11.bemta-5.messagelabs.com id 83/4E-01733-87C19495; Tue, 20 Jun 2017 13:00:40 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrIIsWRWlGSWpSXmKPExsVyMbThkG6JjGe kQdNXLovvWyYzOTB6HP5whSWAMYo1My8pvyKBNaP1xQXWglMmFV8/NbM2MG7Q6GLk4hASmMko cX7qGjYQh0VgKqvE87WfGEEcCYGNrBITzmwEcjiBnDiJF7/7WSHsKolt55eDxYUEVCRubl/FB DHqO6PEwZ57bCAJYQE9iSNHf7BD2B4SF17+A4uzCRhIvNmxF2yQiICSxL1Vk8GamQUuMkq823 8bLMEioCpxYN4aJhCbV8BRYuvbNWDbRAXkJFZebmGFiAtKnJz5hKWLkQOoWVNi/S59kDCzgLz E9rdzmCcwCs1CUjULoWoWkqoFjMyrGNWLU4vKUot0zfSSijLTM0pyEzNzdA0NTPVyU4uLE9NT cxKTivWS83M3MQIDmgEIdjBObXA+xCjJwaQkyqvO4BkpxJeUn1KZkVicEV9UmpNafIhRhoNDS YL3kTRQTrAoNT21Ii0zBxhbMGkJDh4lEd4jEkBp3uKCxNzizHSI1ClGY44rV9Z9YeKYcmD7Fy Yhlrz8vFQpcd4okEkCIKUZpXlwg2Axf4lRVkqYlxHoNCGegtSi3MwSVPlXjOIcjErCvFLABCL Ek5lXArfvFdApTECnvDjiAXJKSSJCSqqBkSMiivGViuBG8U0run0ftilm9a+wOP8qJfH8ic6m Op3MjhD9fyUfpatknCdnT98TWhK1YVlOprPRmQkrhJM+LG4zeD2zJ05+xenj3jun3RB4MMcjd 5fqup1He3qnz+ef3nVkkhVncebaQ6vdlk9c0zkjeyVLygyuLWqlu6SzOptMuj7+qhL+pcRSnJ FoqMVcVJwIALh3h5D0AgAA X-Env-Sender: raistlin.df@gmail.com X-Msg-Ref: server-11.tower-206.messagelabs.com!1497963636!84439771!1 X-Originating-IP: [209.85.128.194] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 9.4.19; banners=-,-,- X-VirusChecked: Checked Received: (qmail 40435 invoked from network); 20 Jun 2017 13:00:36 -0000 Received: from mail-wr0-f194.google.com (HELO mail-wr0-f194.google.com) (209.85.128.194) by server-11.tower-206.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 20 Jun 2017 13:00:36 -0000 Received: by mail-wr0-f194.google.com with SMTP id x23so17315089wrb.0 for ; Tue, 20 Jun 2017 06:00:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:from:to:cc:date:message-id:user-agent:mime-version :content-transfer-encoding; bh=UmWFUyrLX/tlOHTX52WcA/fFlTAe/ZLEn+crQ1U5kao=; b=R8bD5YOzhJ7CLEwArDd82efzSWDhuZiGEFUu03Cqxyvnv+GCw2Mhkk3pR0WS6cyH/2 ua/wUPmqNDgxR+q7wP3wb8EC82u2L8BC9XF0T+jGnwFCW2PVHgrnCnGzU5Ye2+SnZWyo aTpnPIu26/pLe240eDYcdPpDUuxj2DAhZjIsEhPRRBDRR3MzETfD7Dnr99m9XLH7qbzx 75upSwrENXE84dlHVaBBJ9nuDDxyMLxLrhPqd5Kvk5s5k+IM0yqqraBT6MTOVtcDPb5A X9XOLjwyjzSncn2KsUK0QNJD33WkA3mdKSQkMn7NC4kwXZtTPzxBfFDX5Ghu1h4f+zFb n4jQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:from:to:cc:date:message-id :user-agent:mime-version:content-transfer-encoding; bh=UmWFUyrLX/tlOHTX52WcA/fFlTAe/ZLEn+crQ1U5kao=; b=TZStfWPE1rB7OqsLIbiaSgWOmF+eL7ZS3+/idYCim/DCTgIuQbbTRp7KijWQKmUk8P top40uzt9kH97e/owcAhFXdgWgXBGWlrhIaTNf+r/nGbhLcFCNJz2FpASelHuBDTkdbv rQhIzBtVERXTbGI11FZr0UmCQ1u6L4uxfEqaOfxZ+bKUNaTd0mPEOKGbOk+zYJ3OMMOH DYq+JhnXJg5ld5NXO05d3VHwY1tm7vswiCRVQvGYDZaAfb+wmAeoCsCYokJrLbsnzf4A vPpTVe9tqN5a7S04MCuWzIqoWlpPc2nL18C8LTPTkFHrngapySy2gent0DoCZmjff9Rk g51Q== X-Gm-Message-State: AKS2vOxOV6sU0LYxUAGNSa8jzpgeFPpyJ6ldq3SmoNfr0Zlcmj/epiyq MahMMFS5trKJ2A== X-Received: by 10.223.167.15 with SMTP id c15mr22846883wrd.79.1497963635658; Tue, 20 Jun 2017 06:00:35 -0700 (PDT) Received: from [192.168.0.31] ([80.66.223.81]) by smtp.gmail.com with ESMTPSA id l26sm21806630wmi.0.2017.06.20.06.00.33 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 20 Jun 2017 06:00:34 -0700 (PDT) From: Dario Faggioli To: xen-devel@lists.xenproject.org Date: Tue, 20 Jun 2017 15:00:32 +0200 Message-ID: <149796363237.28007.4077302219082749800.stgit@Solace> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Cc: Andrew Cooper , Julien Grall , Stefano Stabellini , Boris Ostrovsky , Jan Beulich Subject: [Xen-devel] [PATCH v2] xen: idle_loop: either deal with tasklets or go idle X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP In fact, there are two kinds of tasklets: vCPU and softirq context. When we want to do vCPU context tasklet work, we force the idle vCPU (of a particular pCPU) into execution, and run it from there. This means there are two possible reasons for choosing to run the idle vCPU: 1) we want a pCPU to go idle, 2) we want to run some vCPU context tasklet work. If we're in case 2), it does not make sense to even try to go idle (as the check will _always_ fail). This patch rearranges the code of the body of idle vCPUs, so that we actually check whether we are in case 1) or 2), and act accordingly. As a matter of fact, this also means that we do not check if there's any tasklet work to do after waking up from idle. This is not a problem, because: a) for softirq context tasklets, if any is queued "during" wakeup from idle, TASKLET_SOFTIRQ is raised, and the call to do_softirq() (which is still happening *after* the wakeup) will take care of it; b) for vCPU context tasklets, if any is queued "during" wakeup from idle, SCHEDULE_SOFTIRQ is raised and do_softirq() (happening after the wakeup) calls the scheduler. The scheduler sees that there is tasklet work pending and confirms the idle vCPU in execution, which then will get to execute do_tasklet(). Signed-off-by: Dario Faggioli Reviewed-by: Jan Beulich Reviewed-by: Stefano Stabellini --- Cc: Andrew Cooper Cc: Julien Grall Cc: Stefano Stabellini Cc: Boris Ostrovsky Cc: Jan Beulich --- Changes from v1: * drop the pointless parentheses and indirection of pm_idle (in x86's idle loop); * remove the 'cpu' input parameter to do_tasklet(), as suggested during review; * in do_tasklet(), convert the check that there is tasklet work to do into an ASSERT() to the same effect, as suggested during review; * add a comment in cpu_is_haltable() on why we check the per-cpu tasklet_work_to_do variable directly, rather than calling the new tasklet_work_to_do() helper. --- xen/arch/arm/domain.c | 21 ++++++++++++++------- xen/arch/x86/domain.c | 12 +++++++++--- xen/common/tasklet.c | 12 ++++++++---- xen/include/xen/sched.h | 5 +++++ xen/include/xen/tasklet.h | 10 ++++++++++ 5 files changed, 46 insertions(+), 14 deletions(-) diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c index 76310ed..2dc8b0a 100644 --- a/xen/arch/arm/domain.c +++ b/xen/arch/arm/domain.c @@ -41,20 +41,27 @@ DEFINE_PER_CPU(struct vcpu *, curr_vcpu); void idle_loop(void) { + unsigned int cpu = smp_processor_id(); + for ( ; ; ) { - if ( cpu_is_offline(smp_processor_id()) ) + if ( cpu_is_offline(cpu) ) stop_cpu(); - local_irq_disable(); - if ( cpu_is_haltable(smp_processor_id()) ) + /* Are we here for running vcpu context tasklets, or for idling? */ + if ( unlikely(tasklet_work_to_do(cpu)) ) + do_tasklet(); + else { - dsb(sy); - wfi(); + local_irq_disable(); + if ( cpu_is_haltable(cpu) ) + { + dsb(sy); + wfi(); + } + local_irq_enable(); } - local_irq_enable(); - do_tasklet(); do_softirq(); /* * We MUST be last (or before dsb, wfi). Otherwise after we get the diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 49388f4..3a061a9 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -112,12 +112,18 @@ static void play_dead(void) static void idle_loop(void) { + unsigned int cpu = smp_processor_id(); + for ( ; ; ) { - if ( cpu_is_offline(smp_processor_id()) ) + if ( cpu_is_offline(cpu) ) play_dead(); - (*pm_idle)(); - do_tasklet(); + + /* Are we here for running vcpu context tasklets, or for idling? */ + if ( unlikely(tasklet_work_to_do(cpu)) ) + do_tasklet(); + else + pm_idle(); do_softirq(); /* * We MUST be last (or before pm_idle). Otherwise after we get the diff --git a/xen/common/tasklet.c b/xen/common/tasklet.c index 365a777..0f0a6f8 100644 --- a/xen/common/tasklet.c +++ b/xen/common/tasklet.c @@ -111,11 +111,15 @@ void do_tasklet(void) struct list_head *list = &per_cpu(tasklet_list, cpu); /* - * Work must be enqueued *and* scheduled. Otherwise there is no work to - * do, and/or scheduler needs to run to update idle vcpu priority. + * We want to be sure any caller has checked that a tasklet is both + * enqueued and scheduled, before calling this. And, if the caller has + * actually checked, it's not an issue that we are outside of the + * critical region, in fact: + * - TASKLET_enqueued is cleared only here, + * - TASKLET_scheduled is only cleared when schedule() find it set, + * without TASKLET_enqueued being set as well. */ - if ( likely(*work_to_do != (TASKLET_enqueued|TASKLET_scheduled)) ) - return; + ASSERT(tasklet_work_to_do(cpu)); spin_lock_irq(&tasklet_lock); diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h index 1127ca9..6673b27 100644 --- a/xen/include/xen/sched.h +++ b/xen/include/xen/sched.h @@ -843,6 +843,11 @@ uint64_t get_cpu_idle_time(unsigned int cpu); /* * Used by idle loop to decide whether there is work to do: * (1) Run softirqs; or (2) Play dead; or (3) Run tasklets. + * + * About (3), if a tasklet is enqueued, it will be scheduled + * really really soon, and hence it's pointless to try to + * sleep between these two events (that's why we don't call + * the tasklet_work_to_do() helper). */ #define cpu_is_haltable(cpu) \ (!softirq_pending(cpu) && \ diff --git a/xen/include/xen/tasklet.h b/xen/include/xen/tasklet.h index 8c3de7e..23d69c7 100644 --- a/xen/include/xen/tasklet.h +++ b/xen/include/xen/tasklet.h @@ -40,6 +40,16 @@ DECLARE_PER_CPU(unsigned long, tasklet_work_to_do); #define TASKLET_enqueued (1ul << _TASKLET_enqueued) #define TASKLET_scheduled (1ul << _TASKLET_scheduled) +static inline bool tasklet_work_to_do(unsigned int cpu) +{ + /* + * Work must be enqueued *and* scheduled. Otherwise there is no work to + * do, and/or scheduler needs to run to update idle vcpu priority. + */ + return per_cpu(tasklet_work_to_do, cpu) == (TASKLET_enqueued| + TASKLET_scheduled); +} + void tasklet_schedule_on_cpu(struct tasklet *t, unsigned int cpu); void tasklet_schedule(struct tasklet *t); void do_tasklet(void);