All of lore.kernel.org
 help / color / mirror / Atom feed
From: Atish Patra <atish.patra@oracle.com>
To: Mike Galbraith <efault@gmx.de>, Peter Zijlstra <peterz@infradead.org>
Cc: linux-kernel@vger.kernel.org, joelaf@google.com,
	brendan.jackman@arm.com, jbacik@fb.com, mingo@redhat.com
Subject: Re: [PATCH RFC 1/2] sched: Minimize the idle cpu selection race window.
Date: Wed, 1 Nov 2017 01:08:59 -0500	[thread overview]
Message-ID: <bc5730d4-b3ee-1fcc-7f57-824db606734f@oracle.com> (raw)
In-Reply-To: <1509439705.14765.16.camel@gmx.de>



On 10/31/2017 03:48 AM, Mike Galbraith wrote:
> On Tue, 2017-10-31 at 09:20 +0100, Peter Zijlstra wrote:
>> On Tue, Oct 31, 2017 at 12:27:41AM -0500, Atish Patra wrote:
>>> Currently, multiple tasks can wakeup on same cpu from
>>> select_idle_sibiling() path in case they wakeup simulatenously
>>> and last ran on the same llc. This happens because an idle cpu
>>> is not updated until idle task is scheduled out. Any task waking
>>> during that period may potentially select that cpu for a wakeup
>>> candidate.
>>>
>>> Introduce a per cpu variable that is set as soon as a cpu is
>>> selected for wakeup for any task. This prevents from other tasks
>>> to select the same cpu again. Note: This does not close the race
>>> window but minimizes it to accessing the per-cpu variable. If two
>>> wakee tasks access the per cpu variable at the same time, they may
>>> select the same cpu again. But it minimizes the race window
>>> considerably.
>> The very most important question; does it actually help? What
>> benchmarks, give what numbers?
Here are the numbers from one of the OLTP configuration on a 8 socket 
x86 machine
kernel          txn/minute (normalized)    user/sys
baseline      1.0                                          80/5
pcpu            1.021                                      84/5

The throughput gains are not very high and close to run-to-run variation %.
The schedstat data (added for testing in 2/2 patch) indicates the there 
are many instances of the
race conditions that got addressed but may be not enough to trigger a 
significant throughput change.

All other benchmark I tested (TPCC, hackbench, schbench, swingbench) did 
not show any regression.

I will let Joel post numbers from Android benchmarks.
> I played with something ~similar (cmpxchg() idle cpu reservation)
I had an atomic version earlier as well. Peter's suggestion for per cpu 
seems to perform slightly better than atomic.
Thus, this patch has the per cpu version.
>   a
> while back in the context of schbench, and it did help that,
Do you have the schbench configuration somewhere that I can test? I 
tried various configurations but did not
see any improvement or regression.
> but for
> generic fast mover benchmarks, the added overhead had the expected
> effect, it shaved throughput a wee bit (rob Peter, pay Paul, repeat).
which benchmark ? Is it hackbench or something else ?
I have not found any regression yet in my testing. I would be happy to 
test if any other benchmark or different configuration
for hackbench.

Regards,
Atish
> I still have the patch lying about in my rubbish heap, but didn't
> bother to save any of the test results.
>
> 	-Mike
>
>

  reply	other threads:[~2017-11-01  6:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-31  5:27 [PATCH RFC 0/2] Fix race window during idle cpu selection Atish Patra
2017-10-31  5:27 ` [PATCH RFC 1/2] sched: Minimize the idle cpu selection race window Atish Patra
2017-10-31  8:20   ` Peter Zijlstra
2017-10-31  8:48     ` Mike Galbraith
2017-11-01  6:08       ` Atish Patra [this message]
2017-11-01  6:54         ` Mike Galbraith
2017-11-01  7:18           ` Mike Galbraith
2017-11-01 16:36             ` Atish Patra
2017-11-01 20:20               ` Mike Galbraith
2017-11-05  0:58     ` Joel Fernandes
2017-11-22  5:23       ` Atish Patra
2017-11-23 10:52         ` Uladzislau Rezki
2017-11-23 13:13           ` Mike Galbraith
2017-11-23 16:00             ` Josef Bacik
2017-11-23 17:40               ` Mike Galbraith
2017-11-23 21:11               ` Atish Patra
2017-11-24 10:26             ` Uladzislau Rezki
2017-11-24 18:46               ` Mike Galbraith
2017-11-26 20:58                 ` Mike Galbraith
2017-11-28  9:34                 ` Uladzislau Rezki
2017-11-28 10:49                   ` Mike Galbraith
2017-11-29 10:41                     ` Uladzislau Rezki
2017-11-29 18:15                       ` Mike Galbraith
2017-11-30 12:30                         ` Uladzislau Rezki
2017-10-31  5:27 ` [PATCH DEBUG 2/2] sched: Add a stat for " Atish Patra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=bc5730d4-b3ee-1fcc-7f57-824db606734f@oracle.com \
    --to=atish.patra@oracle.com \
    --cc=brendan.jackman@arm.com \
    --cc=efault@gmx.de \
    --cc=jbacik@fb.com \
    --cc=joelaf@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.