Received: by 10.223.164.202 with SMTP id h10csp4986218wrb; Tue, 21 Nov 2017 02:57:53 -0800 (PST) X-Google-Smtp-Source: AGs4zMaMI3P+hvwy5pUz71GRWAE5tPQF6NeClGkK/YLxL3ACwjf73DA/Zq4xt6YOxeyqArL1mCG2 X-Received: by 10.101.97.81 with SMTP id o17mr16297526pgv.363.1511261873449; Tue, 21 Nov 2017 02:57:53 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1511261873; cv=none; d=google.com; s=arc-20160816; b=mr8Lh4T1z5PW83HCAe0/UKTP2SRxDAVFvGfDxbtWu2mLkjbqnAFb3u4N3pLBqcnzvv JRwj6HdAqZCd/qH2y20iVtS7sNTLw8ZanSgyqEe5nU43nirYHTGpk3V7NPWgB4QgwPLF vmc9EYK0fcAivA/opHFdr0BPDzK8al2F4PVeEFnFnCuwPU1tjusTUcIbbH/xvjz4sqzh IObOIfAsSK8HZTPPFjpGgpiEObJKSZTksXcRuRVlMg33SFLQwfLyCi/ZL1i5XygE6MJC budxMesyGu2LzE4yQv0usVym89SaBKGqPk/nNKakR5weeLiBBP1015QP6x51zukTmy+g su3Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:mime-version :references:in-reply-to:message-id:subject:cc:to:from:date :arc-authentication-results; bh=jZuztErfHA2lbzAqVGE1/odBhA09PfmZxMx8WwC1dEw=; b=ButCLYNrTXpiXqkNRpkzbDH+lOhb2Zrsd0MSQTl++rSauHgY0NtQnP07vXufKICcmy YfmLsrICvQGNi4kZLuzzlOloC+a6m5HpnJ4p36O5MnD4IS9igjbJge+KtEm/fWR0LSVK qJuGHfNBPwsHXFVq8m9vXyQEv1p73tRvuIe+cabdaKS5+VphtkJaPs+T7qHTCvZEYN2e 92th5yoXmJVLNlxOU0pD5YumPaWwWgt/BydoYwRjXBIEfMmGBEdYOsoje7bVj472NkHJ Rqqrs4AwRbNjgK9MfVmpe2A4LHV4xpUUrEcGDjH9TS5FD9kNR7COYlltO4AiSnwrrnrA bYIA== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id p21si11612100pfi.396.2017.11.21.02.57.42; Tue, 21 Nov 2017 02:57:53 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751957AbdKUK5H (ORCPT + 75 others); Tue, 21 Nov 2017 05:57:07 -0500 Received: from mail.santannapisa.it ([193.205.80.98]:46985 "EHLO mail.santannapisa.it" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751906AbdKUK5G (ORCPT ); Tue, 21 Nov 2017 05:57:06 -0500 Received: from [10.30.3.214] (account l.abeni@santannapisa.it HELO luca) by santannapisa.it (CommuniGate Pro SMTP 6.1.11) with ESMTPSA id 125597194; Tue, 21 Nov 2017 11:57:03 +0100 Date: Tue, 21 Nov 2017 11:56:57 +0100 From: Luca Abeni To: Jiri Kosina Cc: Peter Zijlstra , Daniel Bristot de Oliveira , Juri Lelli , Linus Torvalds , Mathieu Poirier , Mike Galbraith , Steven Rostedt , Thomas Gleixner , linux-kernel@vger.kernel.org Subject: Re: [PATCH] sched/deadline: Use bools for the state flags Message-ID: <20171121115657.1c8cf684@luca> In-Reply-To: References: X-Mailer: Claws Mail 3.13.2 (GTK+ 2.24.30; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hello, On Tue, 21 Nov 2017 11:44:05 +0100 (CET) Jiri Kosina wrote: > From: Jiri Kosina > > Commit > > 799ba82de01e ("sched/deadline: Use C bitfields for the state flags") > > converted state flags into one-bit signed int. Signed one-bit type can be > either 0 or -1, which is going to cause a problem once 1 is assigned to it > and then the value later tested against 1. I think a different fix has just been committed to tip: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=aa5222e92f8000ed3c1c38dddf11c83222aadfb3 Luca > > The current code is okay, as all the checks are (non-)zero check, but I > believe that we'd rather be safe than sorry here; remove the fragility by > converting the state flags to bool. > > This also silences annoying sparse complaints about this very issue when > compiling any code that includes sched.h. > > Fixes: 799ba82de01e ("sched/deadline: Use C bitfields for the state flags") > Cc: luca abeni > Cc: Peter Zijlstra (Intel) > Cc: Daniel Bristot de Oliveira > Cc: Juri Lelli > Cc: Linus Torvalds > Cc: Mathieu Poirier > Cc: Mike Galbraith > Cc: Peter Zijlstra > Cc: Steven Rostedt > Cc: Thomas Gleixner > Signed-off-by: Jiri Kosina > --- > include/linux/sched.h | 8 ++++---- > kernel/sched/core.c | 8 ++++---- > kernel/sched/deadline.c | 32 ++++++++++++++++---------------- > 3 files changed, 24 insertions(+), 24 deletions(-) > > diff --git a/include/linux/sched.h b/include/linux/sched.h > index a5dc7c98b0a2..b19fa1b96726 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -473,10 +473,10 @@ struct sched_dl_entity { > * conditions between the inactive timer handler and the wakeup > * code. > */ > - int dl_throttled : 1; > - int dl_boosted : 1; > - int dl_yielded : 1; > - int dl_non_contending : 1; > + bool dl_throttled; > + bool dl_boosted; > + bool dl_yielded; > + bool dl_non_contending; > > /* > * Bandwidth enforcement timer. Each -deadline task has its > diff --git a/kernel/sched/core.c b/kernel/sched/core.c > index 75554f366fd3..f9bbf0f55e0e 100644 > --- a/kernel/sched/core.c > +++ b/kernel/sched/core.c > @@ -3738,20 +3738,20 @@ void rt_mutex_setprio(struct task_struct *p, struct task_struct *pi_task) > if (dl_prio(prio)) { > if (!dl_prio(p->normal_prio) || > (pi_task && dl_entity_preempt(&pi_task->dl, &p->dl))) { > - p->dl.dl_boosted = 1; > + p->dl.dl_boosted = true; > queue_flag |= ENQUEUE_REPLENISH; > } else > - p->dl.dl_boosted = 0; > + p->dl.dl_boosted = false; > p->sched_class = &dl_sched_class; > } else if (rt_prio(prio)) { > if (dl_prio(oldprio)) > - p->dl.dl_boosted = 0; > + p->dl.dl_boosted = false; > if (oldprio < prio) > queue_flag |= ENQUEUE_HEAD; > p->sched_class = &rt_sched_class; > } else { > if (dl_prio(oldprio)) > - p->dl.dl_boosted = 0; > + p->dl.dl_boosted = false; > if (rt_prio(oldprio)) > p->rt.timeout = 0; > p->sched_class = &fair_sched_class; > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index 2473736c7616..78cb00ae8b2f 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -133,7 +133,7 @@ void dl_change_utilization(struct task_struct *p, u64 new_bw) > rq = task_rq(p); > if (p->dl.dl_non_contending) { > sub_running_bw(p->dl.dl_bw, &rq->dl); > - p->dl.dl_non_contending = 0; > + p->dl.dl_non_contending = false; > /* > * If the timer handler is currently running and the > * timer cannot be cancelled, inactive_task_timer() > @@ -251,7 +251,7 @@ static void task_non_contending(struct task_struct *p) > return; > } > > - dl_se->dl_non_contending = 1; > + dl_se->dl_non_contending = true; > get_task_struct(p); > hrtimer_start(timer, ns_to_ktime(zerolag_time), HRTIMER_MODE_REL); > } > @@ -271,7 +271,7 @@ static void task_contending(struct sched_dl_entity *dl_se, int flags) > add_rq_bw(dl_se->dl_bw, dl_rq); > > if (dl_se->dl_non_contending) { > - dl_se->dl_non_contending = 0; > + dl_se->dl_non_contending = false; > /* > * If the timer handler is currently running and the > * timer cannot be cancelled, inactive_task_timer() > @@ -676,9 +676,9 @@ static void replenish_dl_entity(struct sched_dl_entity *dl_se, > } > > if (dl_se->dl_yielded) > - dl_se->dl_yielded = 0; > + dl_se->dl_yielded = false; > if (dl_se->dl_throttled) > - dl_se->dl_throttled = 0; > + dl_se->dl_throttled = false; > } > > /* > @@ -1051,7 +1051,7 @@ static inline void dl_check_constrained_dl(struct sched_dl_entity *dl_se) > dl_time_before(rq_clock(rq), dl_next_period(dl_se))) { > if (unlikely(dl_se->dl_boosted || !start_dl_timer(p))) > return; > - dl_se->dl_throttled = 1; > + dl_se->dl_throttled = true; > if (dl_se->runtime > 0) > dl_se->runtime = 0; > } > @@ -1154,7 +1154,7 @@ static void update_curr_dl(struct rq *rq) > > throttle: > if (dl_runtime_exceeded(dl_se) || dl_se->dl_yielded) { > - dl_se->dl_throttled = 1; > + dl_se->dl_throttled = true; > __dequeue_task_dl(rq, curr, 0); > if (unlikely(dl_se->dl_boosted || !start_dl_timer(curr))) > enqueue_task_dl(rq, curr, ENQUEUE_REPLENISH); > @@ -1206,7 +1206,7 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer) > if (p->state == TASK_DEAD && dl_se->dl_non_contending) { > sub_running_bw(p->dl.dl_bw, dl_rq_of_se(&p->dl)); > sub_rq_bw(p->dl.dl_bw, dl_rq_of_se(&p->dl)); > - dl_se->dl_non_contending = 0; > + dl_se->dl_non_contending = false; > } > > raw_spin_lock(&dl_b->lock); > @@ -1216,14 +1216,14 @@ static enum hrtimer_restart inactive_task_timer(struct hrtimer *timer) > > goto unlock; > } > - if (dl_se->dl_non_contending == 0) > + if (!dl_se->dl_non_contending) > goto unlock; > > sched_clock_tick(); > update_rq_clock(rq); > > sub_running_bw(dl_se->dl_bw, &rq->dl); > - dl_se->dl_non_contending = 0; > + dl_se->dl_non_contending = false; > unlock: > task_rq_unlock(rq, p, &rf); > put_task_struct(p); > @@ -1492,7 +1492,7 @@ static void yield_task_dl(struct rq *rq) > * it and the bandwidth timer will wake it up and will give it > * new scheduling parameters (thanks to dl_yielded=1). > */ > - rq->curr->dl.dl_yielded = 1; > + rq->curr->dl.dl_yielded = true; > > update_rq_clock(rq); > update_curr_dl(rq); > @@ -1565,7 +1565,7 @@ static void migrate_task_rq_dl(struct task_struct *p) > raw_spin_lock(&rq->lock); > if (p->dl.dl_non_contending) { > sub_running_bw(p->dl.dl_bw, &rq->dl); > - p->dl.dl_non_contending = 0; > + p->dl.dl_non_contending = false; > /* > * If the timer handler is currently running and the > * timer cannot be cancelled, inactive_task_timer() > @@ -2232,7 +2232,7 @@ static void switched_from_dl(struct rq *rq, struct task_struct *p) > * while SCHED_OTHER in the meanwhile. > */ > if (p->dl.dl_non_contending) > - p->dl.dl_non_contending = 0; > + p->dl.dl_non_contending = false; > > /* > * Since this might be the only -deadline task on the rq, > @@ -2563,9 +2563,9 @@ void __dl_clear_params(struct task_struct *p) > dl_se->dl_bw = 0; > dl_se->dl_density = 0; > > - dl_se->dl_throttled = 0; > - dl_se->dl_yielded = 0; > - dl_se->dl_non_contending = 0; > + dl_se->dl_throttled = false; > + dl_se->dl_yielded = false; > + dl_se->dl_non_contending = false; > } > > bool dl_param_changed(struct task_struct *p, const struct sched_attr *attr) > From 1584672668744457461@xxx Tue Nov 21 10:53:44 +0000 2017 X-GM-THRID: 1584672131693330391 X-Gmail-Labels: Inbox,Category Forums,HistoricalUnread