linux-kernel - Re: [PATCH 6/7] sched: Clean up preempt_enable_no

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]

Date:	Thu, 21 Nov 2013 11:10:18 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	Eliezer Tamir <eliezer.tamir@...ux.intel.com>
Cc:	Arjan van de Ven <arjan@...ux.intel.com>, lenb@...nel.org,
	rjw@...ysocki.net, Chris Leech <christopher.leech@...el.com>,
	David Miller <davem@...emloft.net>, rui.zhang@...el.com,
	jacob.jun.pan@...ux.intel.com,
	Mike Galbraith <bitbucket@...ine.de>,
	Ingo Molnar <mingo@...nel.org>, hpa@...or.com,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org
Subject: Re: [PATCH 6/7] sched: Clean up preempt_enable_no_resched() abuse

On Wed, Nov 20, 2013 at 08:02:54PM +0200, Eliezer Tamir wrote:
> IMHO This has been reviewed thoroughly.
> 
> When Ben Hutchings voiced concerns I rewrote the code to use time_after,
> so even if you do get switched over to a CPU where the time is random
> you will at most poll another full interval.
> 
> Linus asked me to remove this since it makes us use two time values
> instead of one. see https://lkml.org/lkml/2013/7/8/345.

I'm not sure I see how this would be true.

So the do_select() code basically does:

  for (;;) {

    /* actual poll loop */

    if (!need_resched()) {
      if (!busy_end) {
	busy_end = now() + busypoll;
	continue;
      }
      if (!((long)(busy_end - now()) < 0))
	continue;
    }

    /* go sleep */

  }

So imagine our CPU0 timebase is 1 minute ahead of CPU1 (60e9 vs 0), and we start by:

  busy_end = now() + busypoll; /* CPU0: 60e9 + d */

but then we migrate to CPU1 and do:

  busy_end - now() /* CPU1: 60e9 + d' */

and find we're still a minute out; and in fact we'll keep spinning for
that entire minute barring a need_resched().

Surely that's not intended and desired?
--
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/