From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 11045171C7 for ; Fri, 22 Dec 2023 10:23:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 94C2D2F4; Fri, 22 Dec 2023 02:24:34 -0800 (PST) Received: from [10.57.87.46] (unknown [10.57.87.46]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 91F953F738; Fri, 22 Dec 2023 02:23:45 -0800 (PST) Message-ID: <11171acd-dae7-475f-ab93-a890f0f0d273@arm.com> Date: Fri, 22 Dec 2023 10:23:43 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v7 19/23] sched: Consolidate pick_*_task to task_is_pushable helper Content-Language: en-US To: John Stultz , LKML Cc: Joel Fernandes , Qais Yousef , Ingo Molnar , Peter Zijlstra , Juri Lelli , Vincent Guittot , Dietmar Eggemann , Valentin Schneider , Steven Rostedt , Ben Segall , Zimuzo Ezeozue , Youssef Esmat , Mel Gorman , Daniel Bristot de Oliveira , Will Deacon , Waiman Long , Boqun Feng , "Paul E. McKenney" , Xuewen Yan , K Prateek Nayak , Thomas Gleixner , kernel-team@android.com References: <20231220001856.3710363-1-jstultz@google.com> <20231220001856.3710363-20-jstultz@google.com> From: Metin Kaya In-Reply-To: <20231220001856.3710363-20-jstultz@google.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 20/12/2023 12:18 am, John Stultz wrote: > From: Connor O'Brien > > This patch consolidates rt and deadline pick_*_task functions to > a task_is_pushable() helper > > This patch was broken out from a larger chain migration > patch originally by Connor O'Brien. > > Cc: Joel Fernandes > Cc: Qais Yousef > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Juri Lelli > Cc: Vincent Guittot > Cc: Dietmar Eggemann > Cc: Valentin Schneider > Cc: Steven Rostedt > Cc: Ben Segall > Cc: Zimuzo Ezeozue > Cc: Youssef Esmat > Cc: Mel Gorman > Cc: Daniel Bristot de Oliveira > Cc: Will Deacon > Cc: Waiman Long > Cc: Boqun Feng > Cc: "Paul E. McKenney" > Cc: Metin Kaya > Cc: Xuewen Yan > Cc: K Prateek Nayak > Cc: Thomas Gleixner > Cc: kernel-team@android.com > Signed-off-by: Connor O'Brien > [jstultz: split out from larger chain migration patch, > renamed helper function] > Signed-off-by: John Stultz > --- > v7: > * Split from chain migration patch > * Renamed function > --- > kernel/sched/deadline.c | 10 +--------- > kernel/sched/rt.c | 11 +---------- > kernel/sched/sched.h | 10 ++++++++++ > 3 files changed, 12 insertions(+), 19 deletions(-) > > diff --git a/kernel/sched/deadline.c b/kernel/sched/deadline.c > index def1eb23318b..1f3bc50de678 100644 > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -2049,14 +2049,6 @@ static void task_fork_dl(struct task_struct *p) > /* Only try algorithms three times */ > #define DL_MAX_TRIES 3 > > -static int pick_dl_task(struct rq *rq, struct task_struct *p, int cpu) > -{ > - if (!task_on_cpu(rq, p) && > - cpumask_test_cpu(cpu, &p->cpus_mask)) > - return 1; > - return 0; > -} > - > /* > * Return the earliest pushable rq's task, which is suitable to be executed > * on the CPU, NULL otherwise: > @@ -2075,7 +2067,7 @@ static struct task_struct *pick_earliest_pushable_dl_task(struct rq *rq, int cpu > if (next_node) { > p = __node_2_pdl(next_node); > > - if (pick_dl_task(rq, p, cpu)) > + if (task_is_pushable(rq, p, cpu) == 1) Nit: ` == 1` part is redundant, IMHO. > return p; > > next_node = rb_next(next_node); > diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c > index cf0eb4aac613..15161de88753 100644 > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -1812,15 +1812,6 @@ static void put_prev_task_rt(struct rq *rq, struct task_struct *p) > /* Only try algorithms three times */ > #define RT_MAX_TRIES 3 > > -static int pick_rt_task(struct rq *rq, struct task_struct *p, int cpu) > -{ > - if (!task_on_cpu(rq, p) && > - cpumask_test_cpu(cpu, &p->cpus_mask)) > - return 1; > - > - return 0; > -} > - > /* > * Return the highest pushable rq's task, which is suitable to be executed > * on the CPU, NULL otherwise > @@ -1834,7 +1825,7 @@ static struct task_struct *pick_highest_pushable_task(struct rq *rq, int cpu) > return NULL; > > plist_for_each_entry(p, head, pushable_tasks) { > - if (pick_rt_task(rq, p, cpu)) > + if (task_is_pushable(rq, p, cpu) == 1) Ditto. > return p; > } > > diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h > index 19afe532771f..ef3d327e267c 100644 > --- a/kernel/sched/sched.h > +++ b/kernel/sched/sched.h > @@ -3554,6 +3554,16 @@ void push_task_chain(struct rq *rq, struct rq *dst_rq, struct task_struct *task) > set_task_cpu(task, dst_rq->cpu); > activate_task(dst_rq, task, 0); > } > + > +static inline > +int task_is_pushable(struct rq *rq, struct task_struct *p, int cpu) Nit: I know the function is just renamed in this patch, but should we change the return type to bool while we are at it? > +{ > + if (!task_on_cpu(rq, p) && > + cpumask_test_cpu(cpu, &p->cpus_mask)) > + return 1; > + > + return 0; > +} > #endif > > #endif /* _KERNEL_SCHED_SCHED_H */