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

[Wanpeng Li <wanpeng.li@linux.intel.com>]

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@linux.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
>