From patchwork Thu May 19 08:11:35 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dario Faggioli X-Patchwork-Id: 9124741 Return-Path: X-Original-To: patchwork-xen-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id D8A7ABF29F for ; Thu, 19 May 2016 08:13:39 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 43F6020218 for ; Thu, 19 May 2016 08:13:34 +0000 (UTC) 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.kernel.org (Postfix) with ESMTPS id 46343201C0 for ; Thu, 19 May 2016 08:13:33 +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 1b3J3U-0005Ac-NI; Thu, 19 May 2016 08:11:40 +0000 Received: from mail6.bemta14.messagelabs.com ([193.109.254.103]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1b3J3T-0005A6-Aq for xen-devel@lists.xenproject.org; Thu, 19 May 2016 08:11:39 +0000 Received: from [193.109.254.147] by server-4.bemta-14.messagelabs.com id F3/37-03277-A357D375; Thu, 19 May 2016 08:11:38 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFvrAIsWRWlGSWpSXmKPExsXiVRvkrGtZaht ucLnXxuL7lslMDowehz9cYQlgjGLNzEvKr0hgzVgxOb1gpXLF6aNbGBsYf0l3MXJxCAnMZJR4 8eQVI4jDIrCGVeL92vcsII6EwCVWifvvXjN3MXICOTESly7vZ4GwqyVOLT7MBGILCahI3Ny+i gli1EImibuTb7CDJIQF9CSOHP0BZHMA2e4SE86mg4TZBAwk3uzYywpiiwgoSdxbNRlsDrNAts SsX2sZQWwWAVWJqfPvM4O08gp4SRydJw0S5hTwlnh/vZMNYq2XxJKud2CniQrISay83AI2kld AUOLkzCcsIK3MApoS63fpQ0yXl9j+dg7zBEaRWUiqZiFUzUJStYCReRWjRnFqUVlqka6hmV5S UWZ6RkluYmaOrqGhiV5uanFxYnpqTmJSsV5yfu4mRmDo1zMwMO5g/Hra8xCjJAeTkijvcX/bc CG+pPyUyozE4oz4otKc1OJDjDIcHEoSvG+KgXKCRanpqRVpmTnAKIRJS3DwKInwHgdJ8xYXJO YWZ6ZDpE4x6nJsmXpvLZMQS15+XqqUOO9SkCIBkKKM0jy4EbCEcIlRVkqYl5GBgUGIpyC1KDe zBFX+FaM4B6OSMO9jkCk8mXklcJteAR3BBHTELTEbkCNKEhFSUg2MseeO+umac0od/vuNo2wx 62S5yCZLrrJHyx51xD7WeuzM8XSO5LrkzI9Gd5Sd6o6+s11oI8xT8rbofWPA9pJ01vAu/+KWX peoz+4ZOz8kdNS6Cmjtm8wapfCU9aHLTw622UXlwka/ukN+avTMU/crX+Ved7umpX9FpVL+d+ mY5I3yIbPvP1FiKc5INNRiLipOBACZ8o1xAwMAAA== X-Env-Sender: raistlin.df@gmail.com X-Msg-Ref: server-7.tower-27.messagelabs.com!1463645497!42498924!1 X-Originating-IP: [74.125.82.67] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.34; banners=-,-,- X-VirusChecked: Checked Received: (qmail 55864 invoked from network); 19 May 2016 08:11:37 -0000 Received: from mail-wm0-f67.google.com (HELO mail-wm0-f67.google.com) (74.125.82.67) by server-7.tower-27.messagelabs.com with AES128-GCM-SHA256 encrypted SMTP; 19 May 2016 08:11:37 -0000 Received: by mail-wm0-f67.google.com with SMTP id n129so18904490wmn.1 for ; Thu, 19 May 2016 01:11:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:from:to:cc:date:message-id:in-reply-to:references :user-agent:mime-version:content-transfer-encoding; bh=FK91AljXYC2xJ1CffloGAVSr8wCjD+ot48mtLnAfbG0=; b=h/u5zElyJQqS0orWdHHP23xlDOZ+gABSJEzVixBwK9WkRl4bmb0jWxhE37Ecc4RNSM 851+VpkB4Fdgb4gWTwH8X7/U7lEGih3DJwl3Y4WcICiRtgU42b37tTLZVoMFtOVhwRe5 0fQf0LGW3iA1m6UcU+Rb72k+IbL5hhZaWyPGi7EeXHsU+MhLqYBzucy373/Fzl9aJXD4 1nT9xW6fMPkmHxQ2b3DOwB5wmwbDDxvoL63y+7x7U5G8OJy3KYdVIQY73ysDmkg6bXtQ VWNY6l868AxchXmYieIp/t1FhXPm4yXQoM0dKK7H9KhiZaE8zwcbplSY/EAkgjlf9MV4 akUQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:from:to:cc:date:message-id :in-reply-to:references:user-agent:mime-version :content-transfer-encoding; bh=FK91AljXYC2xJ1CffloGAVSr8wCjD+ot48mtLnAfbG0=; b=jph1gAmC1HK+bPZvbo+STs1hDjtmjBLVi/q69W5mAZdCct4owTcrtux31U8zxGSJe4 bZzKTNM7CfDqHZRnBhGrSVeSc69sJ+lhdMnaCd3H5qYaRgQb30w8UDVyo/8dTtHFGyMA HNwsyiFEiYrG3QIIElYizLEv6VDg5We3QJzSGnqLDK0zbC+3NspP67f7z6SshAnYqEdh QdmKwHEEKGUAmjhjfpvu6mDKedOTQlVzJUsFjHKm7h8lx0cX+ZtmRc+8MkHCun9Uxka9 ujsb8wTdf89iLaRdEED+M1DE1oK9csiDnrKXVrKSjzUgGxVCXy5ZcmAMZ/OU9AbZCar7 04ig== X-Gm-Message-State: AOPr4FX+IC8DTrVCDllwixSEF1Bm4rnbozY1VEEsQewTz9LwdU9C4QTbH1aHPx45ElFU/g== X-Received: by 10.194.157.161 with SMTP id wn1mr6844822wjb.131.1463645497420; Thu, 19 May 2016 01:11:37 -0700 (PDT) Received: from Solace.fritz.box (net-93-65-141-163.cust.vodafonedsl.it. [93.65.141.163]) by smtp.gmail.com with ESMTPSA id u6sm33897600wmd.21.2016.05.19.01.11.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 19 May 2016 01:11:36 -0700 (PDT) From: Dario Faggioli To: xen-devel@lists.xenproject.org Date: Thu, 19 May 2016 10:11:35 +0200 Message-ID: <146364549356.5283.1620067333752375115.stgit@Solace.fritz.box> In-Reply-To: <146364370619.5283.5027289751613289080.stgit@Solace.fritz.box> References: <146364370619.5283.5027289751613289080.stgit@Solace.fritz.box> User-Agent: StGit/0.17.1-dirty MIME-Version: 1.0 Cc: George Dunlap , Meng Xu , Wei Liu Subject: [Xen-devel] [PATCH] xen: sched: avoid races on time values read from NOW() 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-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED, T_DKIM_INVALID, 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 or (even in cases where there is no race, e.g., outside of Credit2) avoid using a time sample which may be rather old, and hence stale. In fact, we should only sample NOW() from _inside_ the critical region within which the value we read is used. If we don't, in case we have to spin for a while before entering the region, when actually using it: 1) we will use something that, at the veryy least, is not really "now", because of the spinning, 2) if someone else sampled NOW() during a critical region protected by the lock we are spinning on, and if we compare the two samples when we get inside our region, our one will be 'earlier', even if we actually arrived later, which is a race. In Credit2, we see an instance of 2), in runq_tickle(), when it is called by csched2_context_saved() as it samples NOW() before acquiring the runq lock. This makes things look like the time went backwards, and it confuses the algorithm (there's even a d2printk() about it, which would trigger all the time, if enabled). In RTDS, something similar happens in repl_timer_handler(), and there's another instance in schedule() (in generic code), so fix these cases too. While there, improve csched2_vcpu_wake() and and rt_vcpu_wake() a little as well (removing a pointless initialization, and moving the sampling a bit closer to its use). These two hunks entail no further functional changes. Signed-off-by: Dario Faggioli Reviewed-by: George Dunlap Reviewed-by: Meng Xu --- Cc: George Dunlap Cc: Meng Xu Cc: Wei Liu --- xen/common/sched_credit2.c | 4 ++-- xen/common/sched_rt.c | 7 +++++-- xen/common/schedule.c | 4 +++- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c index f95e509..1933ff1 100644 --- a/xen/common/sched_credit2.c +++ b/xen/common/sched_credit2.c @@ -1028,7 +1028,7 @@ static void csched2_vcpu_wake(const struct scheduler *ops, struct vcpu *vc) { struct csched2_vcpu * const svc = CSCHED2_VCPU(vc); - s_time_t now = 0; + s_time_t now; /* Schedule lock should be held at this point. */ @@ -1085,8 +1085,8 @@ static void csched2_context_saved(const struct scheduler *ops, struct vcpu *vc) { struct csched2_vcpu * const svc = CSCHED2_VCPU(vc); - s_time_t now = NOW(); spinlock_t *lock = vcpu_schedule_lock_irq(vc); + s_time_t now = NOW(); BUG_ON( !is_idle_vcpu(vc) && svc->rqd != RQD(ops, vc->processor)); diff --git a/xen/common/sched_rt.c b/xen/common/sched_rt.c index aa3ffd2..0946101 100644 --- a/xen/common/sched_rt.c +++ b/xen/common/sched_rt.c @@ -1198,7 +1198,7 @@ static void rt_vcpu_wake(const struct scheduler *ops, struct vcpu *vc) { struct rt_vcpu * const svc = rt_vcpu(vc); - s_time_t now = NOW(); + s_time_t now; bool_t missed; BUG_ON( is_idle_vcpu(vc) ); @@ -1225,6 +1225,7 @@ rt_vcpu_wake(const struct scheduler *ops, struct vcpu *vc) * If a deadline passed while svc was asleep/blocked, we need new * scheduling parameters (a new deadline and full budget). */ + now = NOW(); missed = ( now >= svc->cur_deadline ); if ( missed ) @@ -1394,7 +1395,7 @@ rt_dom_cntl( * from the replq and does the actual replenishment. */ static void repl_timer_handler(void *data){ - s_time_t now = NOW(); + s_time_t now; struct scheduler *ops = data; struct rt_private *prv = rt_priv(ops); struct list_head *replq = rt_replq(ops); @@ -1406,6 +1407,8 @@ static void repl_timer_handler(void *data){ spin_lock_irq(&prv->lock); + now = NOW(); + /* * Do the replenishment and move replenished vcpus * to the temporary list to tickle. diff --git a/xen/common/schedule.c b/xen/common/schedule.c index 80fea39..5e35310 100644 --- a/xen/common/schedule.c +++ b/xen/common/schedule.c @@ -1320,7 +1320,7 @@ static void vcpu_periodic_timer_work(struct vcpu *v) static void schedule(void) { struct vcpu *prev = current, *next = NULL; - s_time_t now = NOW(); + s_time_t now; struct scheduler *sched; unsigned long *tasklet_work = &this_cpu(tasklet_work_to_do); bool_t tasklet_work_scheduled = 0; @@ -1355,6 +1355,8 @@ static void schedule(void) lock = pcpu_schedule_lock_irq(cpu); + now = NOW(); + stop_timer(&sd->s_timer); /* get policy-specific decision on scheduling... */