Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751735AbdFHODF (ORCPT ); Thu, 8 Jun 2017 10:03:05 -0400 Received: from foss.arm.com ([217.140.101.70]:49328 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751428AbdFHODC (ORCPT ); Thu, 8 Jun 2017 10:03:02 -0400 Date: Thu, 8 Jun 2017 15:02:43 +0100 From: Juri Lelli To: Byungchul Park Cc: peterz@infradead.org, mingo@kernel.org, linux-kernel@vger.kernel.org, juri.lelli@gmail.com, rostedt@goodmis.org, kernel-team@lge.com Subject: Re: [PATCH 2/2] sched/deadline: Don't return invalid cpu in cpudl_maximum_cpu() Message-ID: <20170608140243.omvx62ivewtofk75@e106622-lin> References: <1496388663-29067-1-git-send-email-byungchul.park@lge.com> <1496388663-29067-2-git-send-email-byungchul.park@lge.com> <20170606151225.xlhrnh2usajmlu52@e106622-lin> <20170606234224.GD3623@X58A-UD3R> <20170607001454.GE3623@X58A-UD3R> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170607001454.GE3623@X58A-UD3R> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2353 Lines: 72 On 07/06/17 09:14, Byungchul Park wrote: > On Wed, Jun 07, 2017 at 08:42:24AM +0900, Byungchul Park wrote: > > On Tue, Jun 06, 2017 at 04:12:25PM +0100, Juri Lelli wrote: > > > Hi, > > > > > > On 02/06/17 16:31, Byungchul Park wrote: [...] > > > > > > > > static inline int cpudl_maximum_cpu(struct cpudl *cp) > > > > { > > > > - return cp->elements[0].cpu; > > > > + int cpu = cp->elements[0].cpu; > > > > + return cp->elements[cpu].idx == IDX_INVALID ? -1 : cpu; > > > > > > Mmm, don't we get a WARN from cpumask_check() if we return -1 here? > > > > The function does not return -1 without my patch. > > > > Right? > That's actually my point: with the change you are proposing we will start returning -1 and it looks to me that the WARN will start to fire. What about the below instead (properly splitted in 2 patches I guess, and I'm not sure at all the macro thing is pretty at all) ? --->8--- kernel/sched/cpudeadline.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/kernel/sched/cpudeadline.c b/kernel/sched/cpudeadline.c index fba235c7d026..32e3dcef2b81 100644 --- a/kernel/sched/cpudeadline.c +++ b/kernel/sched/cpudeadline.c @@ -108,11 +108,17 @@ static void cpudl_heapify(struct cpudl *cp, int idx) cpudl_heapify_down(cp, idx); } -static inline int cpudl_maximum(struct cpudl *cp) -{ - return cp->elements[0].cpu; +#define cpudl_maximum(field) \ +static inline int cpudl_maximum_##field \ +(struct cpudl *cp) \ +{ \ + return cp->elements[0].field; \ } +cpudl_maximum(cpu); +cpudl_maximum(dl); +cpudl_maximum(idx); + /* * cpudl_find - find the best (later-dl) CPU in the system * @cp: the cpudl max-heap context @@ -131,9 +137,10 @@ int cpudl_find(struct cpudl *cp, struct task_struct *p, cpumask_and(later_mask, cp->free_cpus, &p->cpus_allowed)) { best_cpu = cpumask_any(later_mask); goto out; - } 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); + } else if (cpudl_maximum_idx(cp) != IDX_INVALID && + cpumask_test_cpu(cpudl_maximum_cpu(cp), &p->cpus_allowed) && + dl_time_before(dl_se->deadline, cpudl_maximum_dl(cp))) { + best_cpu = cpudl_maximum_cpu(cp); if (later_mask) cpumask_set_cpu(best_cpu, later_mask); }