From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753216AbbCQINQ (ORCPT ); Tue, 17 Mar 2015 04:13:16 -0400 Received: from mail-wi0-f178.google.com ([209.85.212.178]:37383 "EHLO mail-wi0-f178.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752073AbbCQINH (ORCPT ); Tue, 17 Mar 2015 04:13:07 -0400 Date: Tue, 17 Mar 2015 09:13:02 +0100 From: Ingo Molnar To: Wanpeng Li Cc: Peter Zijlstra , Juri Lelli , linux-kernel@vger.kernel.org Subject: Re: [PATCH RESEND v10] sched/deadline: support dl task migration during cpu hotplug Message-ID: <20150317081302.GA28462@gmail.com> References: <1426231647-11966-1-git-send-email-wanpeng.li@linux.intel.com> <20150316150101.GA18521@gmail.com> <20150316230110.GA14994@kernel> <20150317080612.GA28235@gmail.com> <20150317075322.GA5431@kernel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150317075322.GA5431@kernel> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Wanpeng Li wrote: > Hi Ingo, > On Tue, Mar 17, 2015 at 09:06:13AM +0100, Ingo Molnar wrote: > > > >* Wanpeng Li wrote: > > > >> Hi Ingo, > >> On Mon, Mar 16, 2015 at 04:01:02PM +0100, Ingo Molnar wrote: > >> >> + > >> >> + /* > >> >> + * If cannot preempt any rq, fallback to pick any > >> >> + * online cpu. > >> > > >> >s/If cannot/If we cannot > >> >s/fallback/fall back > >> > >> Will do. > >> > >> > > >> >> + */ > >> >> + fallback = true; > >> >> + cpu = cpumask_any_and(cpu_active_mask, > >> >> + tsk_cpus_allowed(p)); > >> > > >> >shouldn't be on separate lines - but this is also a sign that the guts > >> > >> Otherwise there is a "WARNING: line over 80 characters". > > > >Yes, but did your reaction to that tool's warning improve the code? I > >don't think so. If do what I suggested and reduce indentation a bit, > >you'll fix the warning _and_ improve the code. Win-win. > > Cool, will do. > > > > >> > of this new code should be in a helper function, not inside > >> > several layers of branches. > >> > >> Do you mean the whole patch should be in a helper function? > > > >Probably. > > Will do. > > > > >> >> + if (cpu >= nr_cpu_ids) { > >> >> + if (dl_bandwidth_enabled()) { > >> >> + /* > >> >> + * Fail to find any suitable cpu. > >> >> + * The task will never come back! > >> >> + */ > >> >> + WARN_ON(1); > >> > > >> > Can this condition happen to users with a non-buggy kernel? > >> > >> What do you prefer? ;-) > > > >That was a yes/no question: can this condition trigger on correctly > >working kernels? > > How about add unlikely() here? Please answer my question: can this condition trigger on correctly working kernels? I think so, but maybe I'm wrong? Yes/no. Thanks, Ingo