From patchwork Tue Sep 2 10:36:08 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juri Lelli X-Patchwork-Id: 36449 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ig0-f199.google.com (mail-ig0-f199.google.com [209.85.213.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id CC6E72032B for ; Tue, 2 Sep 2014 10:36:14 +0000 (UTC) Received: by mail-ig0-f199.google.com with SMTP id l13sf35097378iga.2 for ; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:mime-version:in-reply-to:references :date:message-id:subject:from:to:cc:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe:content-type; bh=EZxYFiU0BvAV9tbOqrv6HPBAcebbjQTCgbO1NUIjC1c=; b=cIlfvesdBfImmvuwAO5vahS5bfHy5R888dlIk53qnZI8HPa7PPXSVu8fCsn5dw8jvx Ds9QH4qPa2OWVjN46fITNG4KUT+LuqJjSX9uLWNW3djATk/to6/j1QwXzK8y/8jBtZnD A58UdhaNEDYC1PN8/mma6rFmNOIW2RZcECRb1QHKgfc6L4y7ZdnTRuWykxKHkcdAXyET UiiVpIXfMzZ2kEMr6TNkodYNBr8aFtSkXTwj3hn+kf9sc7XFZ5BOD652Ug63N6JWnBZ2 Bx9RujU359c+OYxSHEGfeuKHaYe5AC94uyl0aFDEmske76qJXABisr+lIWAdHtQ9VZzi TtWg== X-Gm-Message-State: ALoCoQnvrvP66QaG37iWLzmujzjVDSEEkKXWio5ZU4GUFMUc0GJxRQhAe0KHM9YXqez7Q4b9tK4O X-Received: by 10.182.129.37 with SMTP id nt5mr18479596obb.8.1409654174410; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.103.74 with SMTP id x68ls2399062qge.28.gmail; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) X-Received: by 10.52.129.165 with SMTP id nx5mr20945975vdb.25.1409654174316; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) Received: from mail-vc0-x22f.google.com (mail-vc0-x22f.google.com [2607:f8b0:400c:c03::22f]) by mx.google.com with ESMTPS id h4si2083513vdx.8.2014.09.02.03.36.14 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 02 Sep 2014 03:36:14 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::22f as permitted sender) client-ip=2607:f8b0:400c:c03::22f; Received: by mail-vc0-f175.google.com with SMTP id lf12so6635296vcb.20 for ; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) X-Received: by 10.52.32.66 with SMTP id g2mr235767vdi.49.1409654174200; Tue, 02 Sep 2014 03:36:14 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.45.67 with SMTP id uj3csp505715vcb; Tue, 2 Sep 2014 03:36:13 -0700 (PDT) X-Received: by 10.68.177.68 with SMTP id co4mr46572940pbc.93.1409654172731; Tue, 02 Sep 2014 03:36:12 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id te6si5009515pbc.227.2014.09.02.03.36.12 for ; Tue, 02 Sep 2014 03:36:12 -0700 (PDT) Received-SPF: none (google.com: linux-rt-users-owner@vger.kernel.org does not designate permitted sender hosts) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752518AbaIBKgL (ORCPT + 3 others); Tue, 2 Sep 2014 06:36:11 -0400 Received: from mail-wi0-f176.google.com ([209.85.212.176]:34110 "EHLO mail-wi0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750863AbaIBKgK (ORCPT ); Tue, 2 Sep 2014 06:36:10 -0400 Received: by mail-wi0-f176.google.com with SMTP id bs8so14051945wib.9 for ; Tue, 02 Sep 2014 03:36:08 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.180.211.71 with SMTP id na7mr24749076wic.14.1409654168598; Tue, 02 Sep 2014 03:36:08 -0700 (PDT) Received: by 10.216.180.138 with HTTP; Tue, 2 Sep 2014 03:36:08 -0700 (PDT) In-Reply-To: <871ts0s4pn.fsf@cecht.legt.fr> References: <20140710122033.58bdd366fe3f869a71b1c05e@gmail.com> <871ts0s4pn.fsf@cecht.legt.fr> Date: Tue, 2 Sep 2014 11:36:08 +0100 Message-ID: Subject: Re: set_schedattr + cpuset issue From: Juri Lelli To: Vincent Legout Cc: Kevin Burns , linux-rt-users , juri.lelli@arm.com Sender: linux-rt-users-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-rt-users@vger.kernel.org X-Original-Sender: juri.lelli@gmail.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 2607:f8b0:400c:c03::22f as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=neutral (body hash did not verify) header.i=@; dmarc=fail (p=NONE dis=NONE) header.from=gmail.com Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , Hi Vincent and Kevin, thanks a lot for your report. I was also actually experiencing something that I think is related to your issue, but then I didn't find any time to send out a proper patch :/. Could you please test what I've attached and see if it fixes your problem? Thanks a lot, - Juri On 28 August 2014 22:07, Vincent Legout wrote: > Hello, > > Juri Lelli writes: > >> On Wed, 2 Jul 2014 17:08:47 -0400 >> Kevin Burns wrote: >> >>> Here's the issue: >>> >>> I am able to allocate a bandwidth with a ratio of .1 to two processes using >>> the sched_setattr() system call. >>> >>> I then am able to add said tasks to a cpuset (with one physical processor) >>> using cset. >>> >>> However, when I then try to update the runtime or period of either task, >>> sched_setattr returns a -EBUSY error. >>> >>> Now, if I repeat the above experiment with just one task, I am able to >>> update the runtime or period without issue. I ran trace-cmd and kernelshark >>> to verify that the bandwidths were indeed being updated correctly. That and >>> htop was reporting a higher percentage of CPUusage, which correlated to the >>> ratios of my task's bandwidth. >>> >>> Any ideas as to why cpuset would cause this behaviour? >>> >> >> Could you create a script that I can use to run your setup and reproduce >> the problem? > > Sorry for the delayed answer. I'm working with Kevin and the problem can > be reproduced using the attached files, also available here: > > http://legout.info/~vincent/sd/ > > On a Ubuntu 14.04 system running Linux 3.16, when running run.sh for the > 2nd time, the 2nd call to sched_setattr() returns EBUSY. Uncommenting > line 41 of run.sh fixes this by returning to SCHED_OTHER before moving > the task to the cpuset. > > The problem arises when using both cpusets and SCHED_DEADLINE. The > process described in section 5.1 of the SCHED_DEADLINE documentation > works fine if the process stays on the same cpuset, but I think their > are some issues when moving a process already in the SCHED_DEADLINE > policy from one cpuset to another. > > According to our experiments, it seems that some fields are not updated > during this process, and it thus fails. When a task moves from one > cpuset to another, the total_bw fields of both cpusets doesn't seem to > be updated. Thus, in the next sched_setattr() call, __dl_overflow() > returns 1 because it thinks total_bw is 0 in the new cpuset. Then, > dl_overflow() returns -1 and we have a EBUSY error. > > The total_bw field may also overflow because __dl_clear and __dl_add are > called while the task whose bandwidth is tsk_bw is not in the cpu > represented by dl_b. > > We can get around this by moving the process back to another scheduling > policy before moving it to another cpuset. But we also had to apply the > following patch in order to be sure that the bandwith is always updated > (on top of v3.16). I'd think this condition has been added to skip all > the tests if the bandwith doesn't change. But there is an issue because > then, the total_bw field is not going to be updated for the new cpu. I'd > think the problem comes from the fact that p->dl.dl_bw is not updated > when a task leaves or returns the SCHED_DEADLINE policy. > > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index bc1638b..0df3008 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -2031,9 +2031,6 @@ static int dl_overflow(struct task_struct *p, int policy, > u64 new_bw = dl_policy(policy) ? to_ratio(period, runtime) : 0; > int cpus, err = -1; > > - if (new_bw == p->dl.dl_bw) > - return 0; > - > /* > * Either if a task, enters, leave, or stays -deadline but changes > * its parameters, we may need to update accordingly the total > > I hope the above explanations make sense and I didn't miss anything > trivial. I'd be happy to provide more information or test anything if > needed. > > Thanks, > Vincent > >From 2243d25fbf0df112a83c12ca8a05e4384ea17058 Mon Sep 17 00:00:00 2001 From: Juri Lelli Date: Wed, 11 Jun 2014 16:37:31 +0200 Subject: [PATCH] sched/deadline: clear dl_entity params when setscheduling to different class When a task is using SCHED_DEADLINE and the user setschedules it to a different class its sched_dl_entity static parameters are not cleaned up. This causes a bug if the user sets it back to SCHED_DEADLINE with the same parameters again. The problem resides in the check we perform at the very beginning of dl_overflow(): if (new_bw == p->dl.dl_bw) return 0; This condition is met in the case depicted above, so the function returns and dl_b->total_bw is not updated (the p->dl.dl_bw is not added to it). After this, admission control is broken. This patch fixes the thing, properly clearing static parameters for a task that ceases to use SCHED_DEADLINE. Reported-by: Daniele Alessandrelli Signed-off-by: Juri Lelli Cc: Ingo Molnar Cc: Peter Zijlstra Cc: Juri Lelli Cc: linux-kernel@vger.kernel.org --- kernel/sched/core.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/kernel/sched/core.c b/kernel/sched/core.c index ec1a286..01738b2 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -1776,6 +1776,21 @@ int wake_up_state(struct task_struct *p, unsigned int state) } /* + * This function clears the sched_dl_entity static params. + */ +static void +__dl_clear_params(struct task_struct *p) +{ + struct sched_dl_entity *dl_se = &p->dl; + + dl_se->dl_runtime = 0; + dl_se->dl_deadline = 0; + dl_se->dl_period = 0; + dl_se->flags = 0; + dl_se->dl_bw = 0; +} + +/* * Perform scheduler related setup for a newly forked process p. * p is forked by current. * @@ -1799,10 +1814,7 @@ static void __sched_fork(unsigned long clone_flags, struct task_struct *p) RB_CLEAR_NODE(&p->dl.rb_node); hrtimer_init(&p->dl.dl_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL); - p->dl.dl_runtime = p->dl.runtime = 0; - p->dl.dl_deadline = p->dl.deadline = 0; - p->dl.dl_period = 0; - p->dl.flags = 0; + __dl_clear_params(p); INIT_LIST_HEAD(&p->rt.run_list); @@ -2060,6 +2072,7 @@ static int dl_overflow(struct task_struct *p, int policy, err = 0; } else if (!dl_policy(policy) && task_has_dl_policy(p)) { __dl_clear(dl_b, p->dl.dl_bw); + __dl_clear_params(p); err = 0; } raw_spin_unlock(&dl_b->lock); -- 2.0.4