mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Rohit Jain <rohit.k.jain@oracle.com>
To: Steven Sistare <steven.sistare@oracle.com>,
	Mike Galbraith <efault@gmx.de>,
	linux-kernel@vger.kernel.org
Cc: peterz@infradead.org, mingo@redhat.com, joelaf@google.com,
	jbacik@fb.com, riel@redhat.com, juri.lelli@redhat.com,
	dhaval.giani@oracle.com
Subject: Re: [RFC 1/2] sched: reduce migration cost between faster caches for idle_balance
Date: Thu, 15 Feb 2018 10:07:43 -0800	[thread overview]
Message-ID: <7e6ee88e-8462-b1ab-a7bf-536a2c576c7d@oracle.com> (raw)
In-Reply-To: <dd972024-b3d6-fa53-7cc5-9e71a01c1837@oracle.com>



On 02/15/2018 08:35 AM, Steven Sistare wrote:
> On 2/10/2018 1:37 AM, Mike Galbraith wrote:
>> On Fri, 2018-02-09 at 11:08 -0500, Steven Sistare wrote:
>>>>> @@ -8804,7 +8803,8 @@ static int idle_balance(struct rq *this_rq, struct rq_flags *rf)
>>>>>   		if (!(sd->flags & SD_LOAD_BALANCE))
>>>>>   			continue;
>>>>>   
>>>>> -		if (this_rq->avg_idle < curr_cost + sd->max_newidle_lb_cost) {
>>>>> +		if (this_rq->avg_idle < curr_cost + sd->max_newidle_lb_cost +
>>>>> +		    sd->sched_migration_cost) {
>>>>>   			update_next_balance(sd, &next_balance);
>>>>>   			break;
>>>>>   		}
>>>> Ditto.
>>> The old code did not migrate if the expected costs exceeded the expected idle
>>> time.  The new code just adds the sd-specific penalty (essentially loss of cache
>>> footprint) to the costs.  The for_each_domain loop visit smallest to largest
>>> sd's, hence visiting smallest to largest migration costs (though the tunables do
>>> not enforce an ordering), and bails at the first sd where the total cost is a lose.
>> Hrm..
>>
>> You're now adding a hypothetical cost to the measured cost of running
>> the LB machinery, which implies that the measurement is insufficient,
>> but you still don't say why it is insufficient.  What happens if you
>> don't do that?  I ask, because when I removed the...
>>
>>     this_rq->avg_idle < sysctl_sched_migration_cost
>>
>> ...bits to check removal effect for Peter, the original reason for it
>> being added did not re-materialize, making me wonder why you need to
>> make this cutoff more aggressive.
> The current code with sysctl_sched_migration_cost discourages migration
> too much, per our test results.  Deleting it entirely from idle_balance()
> may be the right solution, or it may allow too much migration and
> cause regressions due to loss of cache warmth on some workloads.
> Rohit's patch deletes it and adds the sd->sched_migration_cost term
> to allow a migration rate that is somewhere in the middle, and is
> logically sound.  It discourages but does not prevent migration between
> nodes, and encourages but does not always allow migration between cores.
> By contrast, setting relax_domain_level to disable SD_BALANCE_NEWIDLE
> at the SD_NUMA level is a big hammer.
>
> I would be perfectly happy if deleting sysctl_sched_migration_cost from
> idle_balance does the trick.  Last week in a different thread you mentioned
> it did not hurt tbench:
>
>>> Mike, do you remember what comes apart when we take
>>> out the sysctl_sched_migration_cost test in idle_balance()?
>> Used to be anything scheduling cross-core heftily suffered, ie pretty
>> much any localhost communication heavy load.  I just tried disabling it
>> in 4.13 though (pre pti cliff), tried tbench, and it made zip squat
>> difference.  I presume that's due to the meanwhile added
>> this_rq->rd->overload and/or curr_cost checks.
> Can you provide more details on the sysbench oltp test that motivated you
> to add sysctl_sched_migration_cost to idle_balance, so Rohit can re-test it?
>     1b9508f6 sched: Rate-limit newidle
>     Rate limit newidle to migration_cost. It's a win for all stages of
>     sysbench oltp tests.
>
> Rohit is running more tests with a patch that deletes
> sysctl_sched_migration_cost from idle_balance, and for his patch but
> with the 5000 usec mistake corrected back to 500 usec.  So far both
> give improvements over the baseline, but for different cases, so we
> need to try more workloads before we draw any conclusions.
>
> Rohit, can you share your data so far?

Results:

In the following results, "Domain based" approach is as mentioned in the
RFC sent out with the values fixed (As pointed out by Mike). "No check" is
the patch where I just remove the check against sysctl_sched_migration_cost

1) Hackbench results on 2 socket, 44 core and 88 threads Intel x86 machine
(lower is better):

+--------------+-----------------+--------------------------+-------------------------+
|              | Without Patch   |Domain Based              |No Check                 |
+------+-------+--------+--------+-----------------+--------+----------------+--------+
|Loops | Groups|Average |%Std Dev|Average          |%Std Dev|Average         |%Std Dev|
+------+-------+--------+--------+-----------------+--------+----------------+--------+
|100000| 4     |9.701   |0.78    |7.971  (+17.84%) | 1.34   |8.919  (+8.07%) |1.07    |
|100000| 8     |17.186  |0.77    |16.712 (+2.76%)  | 0.87   |17.043 (+0.83%) |0.83    |
|100000| 16    |30.378  |0.55    |29.780 (+1.97%)  | 0.38   |29.565 (+2.67%) |0.29    |
|100000| 32    |54.712  |0.54    |53.001 (+3.13%)  | 0.19   |52.158 (+4.67%) |0.22    |
+------+-------+--------+--------+-----------------+--------+----------------+--------+

2) Sysbench MySQL results on 2 socket, 44 core and 88 threads Intel x86
machine (higher is better):

+-------+--------------------+----------------------------+----------------------------+
|       | Without Patch      | Domain based               | No check                   |
+-------+-----------+--------+-------------------+--------+-------------------+--------+
|Num    | Average   |        | Average           |        | Average           |        |
|Threads| throughput|%Std Dev| throughput        |%Std Dev| throughput        |%Std Dev|
+-------+-----------+--------+-------------------+--------+-------------------+--------+
|    8  | 133658.2  | 0.66   | 134909.4 (+0.94%) | 0.94   | 134232.2 (+0.43%) | 1.29   |
|   16  | 266540    | 0.48   | 268253.4 (+0.64%) | 0.64   | 268584.6 (+0.77%) | 0.37   |
|   32  | 466315.6  | 0.15   | 465903.6 (-0.09%) | 0.28   | 468594.2 (+0.49%) | 0.23   |
|   64  | 720039.4  | 0.23   | 725663.8 (+0.78%) | 0.42   | 717253.8 (-0.39%) | 0.36   |
|   72  | 757284.4  | 0.25   | 770693.4 (+1.77%) | 0.29   | 764984.0 (+1.02%) | 0.38   |
|   80  | 807955.6  | 0.22   | 818446.0 (+1.30%) | 0.24   | 831372.2 (+2.90%) | 0.10   |
|   88  | 863173.8  | 0.25   | 870520.4 (+0.85%) | 0.23   | 887049.0 (+2.77%) | 0.56   |
|   96  | 882950.8  | 0.32   | 890775.4 (+0.89%) | 0.40   | 892913.8 (+1.13%) | 0.41   |
|  128  | 895112.6  | 0.13   | 898524.2 (+0.38%) | 0.16   | 901195.0 (+0.68%) | 0.28   |
+-------+-----------+--------+-------------------+--------+-------------------+--------+

Thanks,
Rohit

  parent reply	other threads:[~2018-02-15 18:08 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08 22:19 [RFC 0/2] sched: Make idle_balance smarter about topology Rohit Jain
2018-02-08 22:19 ` [RFC 1/2] sched: reduce migration cost between faster caches for idle_balance Rohit Jain
2018-02-09  3:42   ` Mike Galbraith
2018-02-09 16:08     ` Steven Sistare
2018-02-10  6:37       ` Mike Galbraith
2018-02-15 16:35         ` Steven Sistare
2018-02-15 18:07           ` Mike Galbraith
2018-02-15 18:21             ` Steven Sistare
2018-02-15 18:39               ` Mike Galbraith
2018-02-15 18:07           ` Rohit Jain [this message]
2018-02-16  4:53             ` Mike Galbraith
2018-02-08 22:19 ` [RFC 2/2] Introduce sysctl(s) for the migration costs Rohit Jain
2018-02-09  3:54   ` Mike Galbraith
2018-02-09 16:10     ` Steven Sistare
2018-02-09 17:08       ` Mike Galbraith
2018-02-09 17:33         ` Steven Sistare
2018-02-09 17:50           ` Mike Galbraith
2018-02-12 15:28   ` Peter Zijlstra

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=7e6ee88e-8462-b1ab-a7bf-536a2c576c7d@oracle.com \
    --to=rohit.k.jain@oracle.com \
    --cc=dhaval.giani@oracle.com \
    --cc=efault@gmx.de \
    --cc=jbacik@fb.com \
    --cc=joelaf@google.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@redhat.com \
    --cc=steven.sistare@oracle.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®