From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758481AbdELU5x (ORCPT ); Fri, 12 May 2017 16:57:53 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:37020 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757872AbdELU5w (ORCPT ); Fri, 12 May 2017 16:57:52 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org CD7D66079B Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=jhugo@codeaurora.org Subject: Re: [RFC 1/2] sched/fair: Fix load_balance() affinity redo path To: Peter Zijlstra Cc: Ingo Molnar , linux-kernel@vger.kernel.org, Dietmar Eggemann , Austin Christ , Tyler Baicar References: <1494608498-4538-1-git-send-email-jhugo@codeaurora.org> <1494608498-4538-2-git-send-email-jhugo@codeaurora.org> <20170512204754.GK4626@worktop.programming.kicks-ass.net> From: Jeffrey Hugo Message-ID: <9580a899-0fdb-7c14-c6e5-397f7d52bcf2@codeaurora.org> Date: Fri, 12 May 2017 14:57:50 -0600 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0 MIME-Version: 1.0 In-Reply-To: <20170512204754.GK4626@worktop.programming.kicks-ass.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 5/12/2017 2:47 PM, Peter Zijlstra wrote: > On Fri, May 12, 2017 at 11:01:37AM -0600, Jeffrey Hugo wrote: >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index d711093..8f783ba 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -8219,8 +8219,19 @@ static int load_balance(int this_cpu, struct rq *this_rq, >> >> /* All tasks on this runqueue were pinned by CPU affinity */ >> if (unlikely(env.flags & LBF_ALL_PINNED)) { >> + struct cpumask tmp; > > You cannot have cpumask's on stack. Well, we need a temp variable to store the intermediate values since the cpumask_* operations are somewhat limited, and require a "storage" parameter. Do you have any suggestions to meet all of these requirements? > >> + >> + /* Cpumask of all initially possible busiest cpus. */ >> + cpumask_copy(&tmp, sched_domain_span(env.sd)); >> + cpumask_clear_cpu(env.dst_cpu, &tmp); > > You forgot to mask with cpu_active_mask. cpus == cpu_active_mask, which we compare against below in just a few lines with the cpumask_intersects check. So, no, I don't think we did forget to mask with cpu_active_mask. > >> + >> cpumask_clear_cpu(cpu_of(busiest), cpus); >> - if (!cpumask_empty(cpus)) { >> + /* >> + * Go back to "redo" iff the load-balance cpumask >> + * contains other potential busiest cpus for the >> + * current sched domain. >> + */ >> + if (cpumask_intersects(cpus, &tmp)) { >> env.loop = 0; >> env.loop_break = sched_nr_migrate_break; >> goto redo; -- Jeffrey Hugo Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.