From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752187AbbCWHZM (ORCPT ); Mon, 23 Mar 2015 03:25:12 -0400 Received: from mail-wg0-f51.google.com ([74.125.82.51]:35511 "EHLO mail-wg0-f51.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751853AbbCWHZJ (ORCPT ); Mon, 23 Mar 2015 03:25:09 -0400 Date: Mon, 23 Mar 2015 08:25:04 +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: <20150323072504.GA25184@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> <20150317081302.GA28462@gmail.com> <20150317075945.GA5646@kernel> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150317075945.GA5646@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: > On Tue, Mar 17, 2015 at 09:13:02AM +0100, Ingo Molnar wrote: > > > >* 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? > > I didn't see it happen, I add this by Juri's suggestion, maybe he can > explain more. > > Ping Juri, ;-) I still haven't seen a satisfactory answer to this question. Please don't resend patches without clearing questions raised during review. Thanks, Ingo