From patchwork Tue Apr 29 08:39:53 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Juri Lelli X-Patchwork-Id: 29314 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-yh0-f72.google.com (mail-yh0-f72.google.com [209.85.213.72]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 92752202FE for ; Tue, 29 Apr 2014 08:40:20 +0000 (UTC) Received: by mail-yh0-f72.google.com with SMTP id f73sf11974135yha.11 for ; Tue, 29 Apr 2014 01:40:20 -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:date:from:to:cc:subject:message-id :in-reply-to:references:mime-version:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe:content-type :content-transfer-encoding; bh=4mAm3Ez6AUQdhn8DE3seGmH0db7zm8gOXpTcz4I46lE=; b=E6Yi20BMe1T6Hi4SJLGwrJDiHSZYEXcXrcIN8P7QIV7iXlQ1IBzYISdcv3ETldvNuM Q26Mw6wTAX2z4qGRdsTdskWgXbPhKaaoF3r/c8clVFcHEcyEj4Codlyj2kc4vJy/XmMm irBhzxeffYJ4lJsSeQHcxB4xrfYohmTfmWLMbbpJu1Lq4dGUPrhewlccZY+qS9CXYkXi a3///UaD+fQi+RrpusNmk4xjJzBpaX1RBIATGF3OhkJOfeS4+bOvkhPs2vBG+oURDwCH iOXRKTMV08J1Vlgp92EFB8EzWcNH2K2ckUnItYJ48ur03UmaVhvvM9ALT5wjQ1SLRDCl TewA== X-Gm-Message-State: ALoCoQlVrk/9RGv+/w6geYNjp0IrrftmW6eBLoUwoaGcQirI2WET7lOMa42pmNSk8l7bRtA2kzTq X-Received: by 10.58.245.132 with SMTP id xo4mr14453354vec.11.1398760820113; Tue, 29 Apr 2014 01:40:20 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.80.107 with SMTP id b98ls803qgd.64.gmail; Tue, 29 Apr 2014 01:40:19 -0700 (PDT) X-Received: by 10.52.108.164 with SMTP id hl4mr24397025vdb.25.1398760819848; Tue, 29 Apr 2014 01:40:19 -0700 (PDT) Received: from mail-ve0-x235.google.com (mail-ve0-x235.google.com [2607:f8b0:400c:c01::235]) by mx.google.com with ESMTPS id cx8si582420vec.111.2014.04.29.01.40.19 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 29 Apr 2014 01:40:19 -0700 (PDT) Received-SPF: none (google.com: patch+caf_=patchwork-forward=linaro.org@linaro.org does not designate permitted sender hosts) client-ip=2607:f8b0:400c:c01::235; Received: by mail-ve0-f181.google.com with SMTP id oy12so9194009veb.40 for ; Tue, 29 Apr 2014 01:40:19 -0700 (PDT) X-Received: by 10.52.249.105 with SMTP id yt9mr562292vdc.34.1398760819755; Tue, 29 Apr 2014 01:40:19 -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.220.221.72 with SMTP id ib8csp179349vcb; Tue, 29 Apr 2014 01:40:19 -0700 (PDT) X-Received: by 10.66.250.161 with SMTP id zd1mr9655299pac.136.1398760818723; Tue, 29 Apr 2014 01:40:18 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id fd9si12195729pad.101.2014.04.29.01.40.17 for ; Tue, 29 Apr 2014 01:40:18 -0700 (PDT) Received-SPF: none (google.com: linux-kernel-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 S933341AbaD2IkA (ORCPT + 28 others); Tue, 29 Apr 2014 04:40:00 -0400 Received: from mail-qc0-f178.google.com ([209.85.216.178]:58575 "EHLO mail-qc0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751675AbaD2Ij5 (ORCPT ); Tue, 29 Apr 2014 04:39:57 -0400 Received: by mail-qc0-f178.google.com with SMTP id i8so8033285qcq.37 for ; Tue, 29 Apr 2014 01:39:56 -0700 (PDT) X-Received: by 10.140.106.66 with SMTP id d60mr38642582qgf.44.1398760796774; Tue, 29 Apr 2014 01:39:56 -0700 (PDT) Received: from pedro (HSI-KBW-37-209-96-85.hsi15.kabel-badenwuerttemberg.de. [37.209.96.85]) by mx.google.com with ESMTPSA id t5sm29144738qas.10.2014.04.29.01.39.54 for (version=TLSv1.1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Tue, 29 Apr 2014 01:39:55 -0700 (PDT) Date: Tue, 29 Apr 2014 10:39:53 +0200 From: Juri Lelli To: "yjay.kim" Cc: Ingo Molnar , Peter Zijlstra , linux-kernel@vger.kernel.org Subject: Re: [PATCH] sched/deadline: remove dl_new checking condition from dl_task_timer() Message-Id: <20140429103953.e68eba1b2ac3309214e3dc5a@gmail.com> In-Reply-To: <1398653845-22615-1-git-send-email-yjay.kim@lge.com> References: <1398653845-22615-1-git-send-email-yjay.kim@lge.com> X-Mailer: Sylpheed 3.2.0beta5 (GTK+ 2.24.10; x86_64-pc-linux-gnu) Mime-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Original-Sender: juri.lelli@gmail.com X-Original-Authentication-Results: mx.google.com; spf=neutral (google.com: patch+caf_=patchwork-forward=linaro.org@linaro.org does not designate permitted sender hosts) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org; dkim=neutral (body hash did not verify) header.i=@gmail.com; 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, On Mon, 28 Apr 2014 11:57:25 +0900 "yjay.kim" wrote: > From: "yjay.kim" > > yield_task_dl() sets dl.dl_new as 1 and dequeue current dl task. > After that it expects that next bandwidth timer callback `dl_task_timer()` will > replenish budget of dl task and enqueue it again. > > But current dl_task_timer() does nothing in case dl.dl_new is 1. > So when dl task calls sched_yield(), it will never be scheduled again. > > dl_task_timer() should works in case dl_new is 1. > > Signed-off-by: yjay.kim > --- > kernel/sched/deadline.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index 27ef409..6fb4004 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -522,7 +522,7 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer) > * different from SCHED_DEADLINE or changed its reservation > * parameters (through sched_setscheduler()). > */ > - if (!dl_task(p) || dl_se->dl_new) Unfortunately, we still need this check to discriminate cases when the user wanted to change reservation parameters while the task was throttled; and we don't want to do anything in this case. > + if (!dl_task(p)) > goto unlock; > > sched_clock_tick(); I'd propose something like the following instead. Thanks, - Juri >From 5c7fb103db06bc8f30a24465000b1c49c7a6310e Mon Sep 17 00:00:00 2001 From: Juri Lelli Date: Tue, 15 Apr 2014 13:49:04 +0200 Subject: [PATCH] sched/deadline: fix sched_yield behavior yield_task_dl() is broken: o it forces current to be throttled setting its runtime to zero; o it sets current's dl_se->dl_new to one, expecting that dl_task_timer() will queue it back with proper parameters at replenish time. Unfortunately, dl_task_timer() has this check at the very beginning: if (!dl_task(p) || dl_se->dl_new) goto unlock; So, it just bails out and the task is never replenished. It actually yielded forever. To fix this, introduce a new flag indicating that the task properly yielded the CPU before its current runtime expired. While this is a little overdoing at the moment, the flag would be useful in the future to discriminate between "good" jobs (of which remaining runtime could be reclaimed, i.e. recycled) and "bad" jobs (for which dl_throttled task has been set) that needed to be stopped. Reported-by: yjay.kim Signed-off-by: Juri Lelli --- include/linux/sched.h | 7 +++++-- kernel/sched/core.c | 1 + kernel/sched/deadline.c | 5 +++-- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/include/linux/sched.h b/include/linux/sched.h index 25f54c7..2a4298f 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1153,9 +1153,12 @@ struct sched_dl_entity { * * @dl_boosted tells if we are boosted due to DI. If so we are * outside bandwidth enforcement mechanism (but only until we - * exit the critical section). + * exit the critical section); + * + * @dl_yielded tells if task gave up the cpu before consuming + * all its available runtime during the last job. */ - int dl_throttled, dl_new, dl_boosted; + int dl_throttled, dl_new, dl_boosted, dl_yielded; /* * Bandwidth enforcement timer. Each -deadline task has its diff --git a/kernel/sched/core.c b/kernel/sched/core.c index 268a45e..a0f5923 100644 --- a/kernel/sched/core.c +++ b/kernel/sched/core.c @@ -3124,6 +3124,7 @@ __setparam_dl(struct task_struct *p, const struct sched_attr *attr) dl_se->dl_bw = to_ratio(dl_se->dl_period, dl_se->dl_runtime); dl_se->dl_throttled = 0; dl_se->dl_new = 1; + dl_se->dl_yielded = 0; } static void __setscheduler_params(struct task_struct *p, diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c index b080957..800e99b 100644 --- a/kernel/sched/deadline.c +++ b/kernel/sched/deadline.c @@ -528,6 +528,7 @@ static enum hrtimer_restart dl_task_timer(struct hrtimer *timer) sched_clock_tick(); update_rq_clock(rq); dl_se->dl_throttled = 0; + dl_se->dl_yielded = 0; if (p->on_rq) { enqueue_task_dl(rq, p, ENQUEUE_REPLENISH); if (task_has_dl_policy(rq->curr)) @@ -893,10 +894,10 @@ static void yield_task_dl(struct rq *rq) * We make the task go to sleep until its current deadline by * forcing its runtime to zero. This way, update_curr_dl() stops * it and the bandwidth timer will wake it up and will give it - * new scheduling parameters (thanks to dl_new=1). + * new scheduling parameters (thanks to dl_yielded=1). */ if (p->dl.runtime > 0) { - rq->curr->dl.dl_new = 1; + rq->curr->dl.dl_yielded = 1; p->dl.runtime = 0; } update_curr_dl(rq);