From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756702AbaKTO6H (ORCPT ); Thu, 20 Nov 2014 09:58:07 -0500 Received: from mail-ie0-f176.google.com ([209.85.223.176]:48095 "EHLO mail-ie0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754574AbaKTO6F (ORCPT ); Thu, 20 Nov 2014 09:58:05 -0500 MIME-Version: 1.0 In-Reply-To: <20141119112451.6fef30f0@gandalf.local.home> References: <1416411982-1204-1-git-send-email-pang.xunlei@linaro.org> <1416411982-1204-3-git-send-email-pang.xunlei@linaro.org> <20141119112451.6fef30f0@gandalf.local.home> Date: Thu, 20 Nov 2014 22:58:03 +0800 Message-ID: Subject: Re: [PATCH v2 3/4] sched/deadline: add the "set_flag" argument to cpudl_find() From: "pang.xunlei" To: Steven Rostedt Cc: lkml , Peter Zijlstra , Juri Lelli Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 20 November 2014 00:24, Steven Rostedt wrote: > On Wed, 19 Nov 2014 23:46:21 +0800 > "pang.xunlei" wrote: > >> The call site of cpudl_find() in check_preempt_equal_dl() doesn't >> use later_mask, so add this extra argument to distinquish the case. >> >> Signed-off-by: pang.xunlei >> --- >> kernel/sched/cpudeadline.c | 6 ++++-- >> kernel/sched/cpudeadline.h | 2 +- >> kernel/sched/deadline.c | 6 +++--- >> 3 files changed, 8 insertions(+), 6 deletions(-) >> >> diff --git a/kernel/sched/cpudeadline.c b/kernel/sched/cpudeadline.c >> index c01b3aa..3047846 100644 >> --- a/kernel/sched/cpudeadline.c >> +++ b/kernel/sched/cpudeadline.c >> @@ -98,11 +98,12 @@ static inline int cpudl_maximum(struct cpudl *cp) >> * @cp: the cpudl max-heap context >> * @p: the task >> * @later_mask: a mask to fill in with the selected CPUs (not NULL) >> + * @set_flag: indicate if later_mask should be set >> * >> * Returns: int - best CPU (heap maximum if suitable) >> */ >> int cpudl_find(struct cpudl *cp, struct task_struct *p, >> - struct cpumask *later_mask) >> + struct cpumask *later_mask, int set_flag) > > set_flag should be a bool type. > >> { >> int best_cpu = -1; >> const struct sched_dl_entity *dl_se = &p->dl; >> @@ -114,7 +115,8 @@ int cpudl_find(struct cpudl *cp, struct task_struct *p, >> } else if (cpumask_test_cpu(cpudl_maximum(cp), &p->cpus_allowed) && >> dl_time_before(dl_se->deadline, cp->elements[0].dl)) { >> best_cpu = cpudl_maximum(cp); >> - cpumask_set_cpu(best_cpu, later_mask); >> + if (set_flag) >> + cpumask_set_cpu(best_cpu, later_mask); > > I'm not sure this is worth it. cpumask_set_cpu() is rather efficient. HI Steve, Thanks for your commenting, I've rethinked this a bit. We can do a little trick with its return value, then could avoid this extra cpumask_set_cpu() without this extra set_flag: 1) define macros for the return values of cpudl_find(), like: #define CPUDL_FIND_NONE -2 /* no available cpus */ #define CPUDL_FIND_CPUMASK -1 /* available cpus in later_mask */ then, with the return value >=0, means it returns the only one available cpu. 2) In the leg of "if", it can just return CPUDL_FIND_CPUMASK, as we want to select the best_cpu in find_later_rq(). In the leg of "else if", just returns cpudl_maximum(cp), apparently there is no need to set the later_mask, since we will definitely select this cpu as the best_cpu in find_later_rq() . int cpudl_find(struct cpudl *cp, struct task_struct *p, struct cpumask *later_mask) { const struct sched_dl_entity *dl_se = &p->dl; cpumask_and(later_mask, &p->cpus_allowed, &p->cpus_allowed); if (cpumask_and(later_mask, later_mask, cp->free_cpus)) { return CPUDL_FIND_CPUMASK; } else if (cpumask_test_cpu(cpudl_maximum(cp), &p->cpus_allowed) && dl_time_before(dl_se->deadline, cp->elements[0].dl)) int cpu; cpu = cpudl_maximum(cp); WARN_ON(!cpu_present(cpu)); return cpu; } out: return CPUDL_FIND_NONE; } Thus, in find_later_rq() we can change the call site code like: best_cpu = cpudl_find(&task_rq(task)->rd->cpudl, task, later_mask); if (best_cpu == CPUDL_FIND_NONE) return -1; if (best_cpu != CPUDL_FIND_CPUMASK) return best_cpu; /* adjust the following code as that in RT find_lowest_rq(), omit here... */ What's your view about this? Thanks, Xunlei > >> } >> >> out: >