linux-kernel - Re: [PATCH v2 1/6] sched/rt: check if curr can be pushed/pulled somewhere else in advance

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20141030215915.GA7478@kernel>
Date:	Fri, 31 Oct 2014 05:59:15 +0800
From:	Wanpeng Li <wanpeng.li@...ux.intel.com>
To:	Kirill Tkhai <ktkhai@...allels.com>
Cc:	Ingo Molnar <mingo@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Juri Lelli <juri.lelli@....com>, juri.lelli@...il.com,
	linux-kernel@...r.kernel.org,
	Wanpeng Li <wanpeng.li@...ux.intel.com>
Subject: Re: [PATCH v2 1/6] sched/rt: check if curr can be pushed/pulled
 somewhere else in advance

Hi Kirill,
On Thu, Oct 30, 2014 at 06:01:05PM +0300, Kirill Tkhai wrote:
>Hi,
>
>В Чт, 30/10/2014 в 22:16 +0800, Wanpeng Li пишет:
>> This patch checks if current can be pushed/pulled somewhere else 
>> in advance to make logic clear, the same behavior as rt class.
>> 
>> - If current can't be migrated, useless to reschedule, let's hope 
>>   task can move out.
>> - If task is migratable, so let's not schedule it and see if it 
>>   can be pushed or pulled somewhere else.
>
>No functional changes, are they? So I'd suggest to change subject
>on something connected with "cleanup".
>
>Also, the description confuses. Please, write that code was cleaned
>up, and that comments were added.
>

Ok.

>> Signed-off-by: Wanpeng Li <wanpeng.li@...ux.intel.com>
>> ---
>>  kernel/sched/rt.c | 14 ++++++++++----
>>  1 file changed, 10 insertions(+), 4 deletions(-)
>> 
>> diff --git a/kernel/sched/rt.c b/kernel/sched/rt.c
>> index d024e6c..a0b51aa 100644
>> --- a/kernel/sched/rt.c
>> +++ b/kernel/sched/rt.c
>> @@ -1351,16 +1351,22 @@ out:
>>  
>>  static void check_preempt_equal_prio(struct rq *rq, struct task_struct *p)
>>  {
>> -	if (rq->curr->nr_cpus_allowed == 1)
>> +	/*
>> +	 * Current can't be migrated, useless to reschedule,
>> +	 * let's hope p can move out.
>> +	 */
>> +	if (rq->curr->nr_cpus_allowed == 1 ||
>> +		!cpupri_find(&rq->rd->cpupri, rq->curr, NULL))
>>  		return;
>
>All conditions of multi-line if() should be aligned the same:
>
>+	if (rq->curr->nr_cpus_allowed == 1 ||
>+	    !cpupri_find(&rq->rd->cpupri, rq->curr, NULL))
>

I will update the patch in next version, thanks for your review.

Regards,
Wanpeng Li

>>  
>> +	/*
>> +	 * p is migratable, so let's not schedule it and
>> +	 * see if it is pushed or pulled somewhere else.
>> +	 */
>>  	if (p->nr_cpus_allowed != 1
>>  	    && cpupri_find(&rq->rd->cpupri, p, NULL))
>>  		return;
>>  
>> -	if (!cpupri_find(&rq->rd->cpupri, rq->curr, NULL))
>> -		return;
>> -
>>  	/*
>>  	 * There appears to be other cpus that can accept
>>  	 * current and none to run 'p', so lets reschedule
>
>Thanks,
>Kirill
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/