Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [PATCH 08/10] sched/fair: Steal work from an overloaded CPU when
 CPU goes idle
To:     Valentin Schneider <valentin.schneider@arm.com>, mingo@redhat.com,
        peterz@infradead.org
Cc:     subhra.mazumdar@oracle.com, dhaval.giani@oracle.com,
        rohit.k.jain@oracle.com, daniel.m.jordan@oracle.com,
        pavel.tatashin@microsoft.com, matt@codeblueprint.co.uk,
        umgwanakikbuti@gmail.com, riel@redhat.com, jbacik@fb.com,
        juri.lelli@redhat.com, linux-kernel@vger.kernel.org
References: <1540220381-424433-1-git-send-email-steven.sistare@oracle.com>
 <1540220381-424433-9-git-send-email-steven.sistare@oracle.com>
 <223bfcdb-bb0c-b25e-04dc-26226a7c3ab3@arm.com>
From:   Steven Sistare <steven.sistare@oracle.com>
Organization: Oracle Corporation
Message-ID: <b8bec531-125b-065f-9160-1df03cc25a9e@oracle.com>
Date:   Thu, 25 Oct 2018 10:07:58 -0400
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:52.0) Gecko/20100101
 Thunderbird/52.9.1
MIME-Version: 1.0
In-Reply-To: <223bfcdb-bb0c-b25e-04dc-26226a7c3ab3@arm.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 10/25/2018 9:48 AM, Valentin Schneider wrote:
> Hi Steve,
> 
> On 22/10/2018 15:59, Steve Sistare wrote:
> [...]
>> +/*
>> + * Try to steal a runnable CFS task from a CPU in the same LLC as @dst_rq,
>> + * and migrate it to @dst_rq.  rq_lock is held on entry and return, but
>> + * may be dropped in between.  Return 1 on success, 0 on failure, and -1
>> + * if a task in a different scheduling class has become runnable on @dst_rq.
>> + */
>> +static int try_steal(struct rq *dst_rq, struct rq_flags *dst_rf)
>> +{
>> +	int src_cpu;
>> +	int dst_cpu = dst_rq->cpu;
>> +	bool locked = true;
>> +	int stolen = 0;
>> +	struct sparsemask *overload_cpus;
>> +
>> +	if (!sched_feat(STEAL))
>> +		return 0;
>> +
>> +	if (!cpu_active(dst_cpu))
>> +		return 0;
>> +
>> +	/* Get bitmap of overloaded CPUs in the same LLC as @dst_rq */
>> +
>> +	rcu_read_lock();
>> +	overload_cpus = rcu_dereference(dst_rq->cfs_overload_cpus);
>> +	if (!overload_cpus) {
>> +		rcu_read_unlock();
>> +		return 0;
>> +	}
>> +
>> +#ifdef CONFIG_SCHED_SMT
>> +	/*
>> +	 * First try overloaded CPUs on the same core to preserve cache warmth.
>> +	 */
>> +	if (static_branch_likely(&sched_smt_present)) {
>> +		for_each_cpu(src_cpu, cpu_smt_mask(dst_cpu)) {
>> +			if (sparsemask_test_elem(src_cpu, overload_cpus) &&
>> +			    steal_from(dst_rq, dst_rf, &locked, src_cpu)) {
>> +				stolen = 1;
>> +				goto out;
>> +			}
>> +		}
>> +	}
>> +#endif	/* CONFIG_SCHED_SMT */
>> +
>> [...]
>> +
>> +	for_each_sparse_wrap(src_cpu, overload_cpus, dst_cpu) {
>> +		if (steal_from(dst_rq, dst_rf, &locked, src_cpu)) {
>> +			stolen = 1;
>> +			break;
> 			^^^^^^
> You might want to have a 'goto out' there for consistency and to make GCC
> happy for !CONFIG_SCHED_SMT (I get a "warning: label ‘out’ defined but not
> used")

Absolutely, will fix in next version.  Thanks for the careful review.

- Steve