linux-kernel - Re: [PATCH] sched: Get rid of unnecessary checks from select_idle

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <50EE798F.4080108@linux.vnet.ibm.com>
Date:	Thu, 10 Jan 2013 13:49:27 +0530
From:	Preeti U Murthy <preeti@...ux.vnet.ibm.com>
To:	Namhyung Kim <namhyung@...nel.org>
CC:	Ingo Molnar <mingo@...nel.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	LKML <linux-kernel@...r.kernel.org>,
	Namhyung Kim <namhyung.kim@....com>,
	Mike Galbraith <efault@....de>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Alex Shi <alex.shi@...el.com>
Subject: Re: [PATCH] sched: Get rid of unnecessary checks from select_idle_sibling

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

--
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/