Re: [PATCH] sched: Get rid of unnecessary checks from select_idle_sibling

[Preeti U Murthy <preeti@linux.vnet.ibm.com>]

On 01/10/2013 11:19 AM, Namhyung Kim wrote:
> Hi Preeti,
> 
> On Wed, 09 Jan 2013 13:51:00 +0530, Preeti U. Murthy wrote:
>> On 01/09/2013 12:20 PM, Namhyung Kim wrote:
>>> From: Namhyung Kim <namhyung.kim@lge.com>
>>>
>>> AFAICS @target cpu of select_idle_sibling() is always either prev_cpu
>>> or this_cpu.  So no need to check it again and the conditionals can be
>>> consolidated.
> [snip]
>> If NUMA_TTWU_BIAS or NUMA_TTWU_TO is true(it is false by
> 
> I can't find those bits in the code.  I've checked v3.8-rc2,
> next-20130110, tip/master and tip/numa/core but there's nothing like
> above.  Which tree are you saying?
> 
> 
>> default),cpu/prev_cpu can be changed to be a random node_cpu(the node
>> that 'this_cpu' is on). In which case even if the node cpu is idle,it
>> would not be a viable target,looks like.Maybe that is why
>> select_idle_sibling() makes the check if the target is prev_cpu/this cpu.
> 
> Looking into tip/numa/core, I can see that there's a code added for
> CONFIG_NUMA_BALANCING.  But still, it seems nothing changed on a path
> from select_task_rq_fair() to select_idle_sibling() - i.e. if the
> select_idle_sibling called, the target would be either prev_cpu or this
> cpu.  Am I missing something?

Hi Namhyung,
Sorry about this.I did a git pull on the tip/master.You are right this
piece of code is not there any more.So not an issue.

You can go ahead and add my Reviewed-by.I don't find any potential
problems now.

For your information however, the code piece which was earlier there, is
given below which aimed to pull the tasks towards the node of the waking
cpu,if NUMA_TTWU_BIAS is enabled.

   if (sched_feat_numa(NUMA_TTWU_BIAS) && node != -1) {
               /*
                * For fork,exec find the idlest cpu in the home-node.
                */
               if (sd_flag & (SD_BALANCE_FORK|SD_BALANCE_EXEC)) {
                       int node_cpu = numa_select_node_cpu(p, node);
                       if (node_cpu < 0)
                               goto find_sd;

                       new_cpu = cpu = node_cpu;
                       sd = per_cpu(sd_node, cpu);
                       goto pick_idlest;
               }

               /*
                * For wake, pretend we were running in the home-node.
                */
               if (cpu_to_node(prev_cpu) != node) {
                       int node_cpu = numa_select_node_cpu(p, node);
                       if (node_cpu < 0)
                               goto find_sd;

                       if (sched_feat_numa(NUMA_TTWU_TO))
                               cpu = node_cpu;
                       else
                               prev_cpu = node_cpu;
               }
       }

> 
> Thanks,
> Namhyung
> 
I apologise once again for the wrong feedback.

Thank you

Regards
Preeti U Murthy