Re: [PATCH v4 04/20] cpumask: Introduce cpu_preferred_mask

[Shrikanth Hegde <sshegde@linux.ibm.com>]

Hi Yury.

>> +void set_cpu_preferred(unsigned int cpu, bool preferred)
>> +{
>> +	if (!IS_ENABLED(CONFIG_PREFERRED_CPU))
>> +		return;
>> +
>> +	assign_cpu((cpu), &__cpu_preferred_mask, (preferred));
>> +}
> 
> set_cpu_xxx() is a macro on purpose - it improves code generation
> quite a lot. See 5c563ee90a22d. Can you keep set_cpu_preferred aligned
> with the other set_cpu(), i.e. make it a macro?
>

Ok. only reason was to avoid ifdeffery there.

you mean below?

#ifdef CONFIG_PREFERRED_CPU
#else
#endif


  
>> +
>>   /*
>>    * Activate the first processor.
>>    */
>> @@ -3164,6 +3177,7 @@ void __init boot_cpu_init(void)
>>   	/* Mark the boot cpu "present", "online" etc for SMP and UP case */
>>   	set_cpu_online(cpu, true);
>>   	set_cpu_active(cpu, true);
>> +	set_cpu_preferred(cpu, true);
>>   	set_cpu_present(cpu, true);
>>   	set_cpu_possible(cpu, true);
>>   
>> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
>> index 2f4530eb543f..9e16946c9d62 100644
>> --- a/kernel/sched/core.c
>> +++ b/kernel/sched/core.c
>> @@ -8685,6 +8685,9 @@ int sched_cpu_activate(unsigned int cpu)
>>   	 */
>>   	sched_set_rq_online(rq, cpu);
>>   
>> +	/* preferred is subset of active and follows its state */
>> +	set_cpu_preferred(cpu, true);
> 
> Did you put it at the end of the function on purpose? If yes, please
> add a comment. If no, I'd prefer to have it immediately after
> set_cpu_active().

         /*
          * Put the rq online, if not already. This happens:
          *
          * 1) In the early boot process, because we build the real domains
          *    after all CPUs have been brought up.

          * 2) At runtime, if cpuset_cpu_active() fails to rebuild the
          *    domains.
          */
         sched_set_rq_online(rq, cpu);


This was the reason. I wanted active related functionality to complete.

> 
>> +
>>   	return 0;
>>   }
>>   
>> @@ -8698,6 +8701,8 @@ int sched_cpu_deactivate(unsigned int cpu)
>>   	if (ret)
>>   		return ret;
>>   
>> +	set_cpu_preferred(cpu, false);
>> +
>>   	/*
>>   	 * Remove CPU from nohz.idle_cpus_mask to prevent participating in
>>   	 * load balancing when not active
>> -- 
>> 2.47.3