[for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Dario Faggioli]

Hi,

This patch changes the way in which runqueue are arranged by default in
Credit2. Despite being so late, I think this should go in Xen 4.8, and here's
why.

So, first of all, runqueues in Credit2 can be arranged in various ways, just by
means of a Xen boot parameter. Right now, the default is one runqueue per-core.
I think we should switch that to one runqueue per-socket.

In fact, all the testing and the benchmarking that I have done before sending
all the various patch series about Credit2 within this release cycle, happened
with per-socket runqueues.

The numbers reported here, for instance, show how per-socket is better:

https://lists.xen.org/archives/html/xen-devel/2016-06/msg02287.html

Pretty much all the other results we have tell the same.

Most of Anshul's benchmarks, the ones he presented at XenSummit, the ones he
has run after that, and the ones he is running now, use per-socket runqueues.

http://www.slideshare.net/xen_com_mgr/xpds16-scope-and-performance-of-credit2-scheduler-anshul-makkar-ctirix-systems-uk-ltd

The reason why we did go for per-core in 4.7, was to introduce some form of
hyperthreading support. Now, we have hyperthreading support, independently from
how runqueues are organized (9bb9c7388 "xen: credit2: implement true SMT
support").

So, in summary, the per-socket ruqneueue configuration is _BOTH_ more tested,
and provide better performance.

The reason why I'm proposing this only now is basically because I thought I had
sent a patch for that already, but I've recently discovered that I must have
took such patch off from one of the series at the last minute. And since then,
I've always forgot to double check... :-(

As it is easy to check, the change is entirely self contained within Credit2,
and bears no risks.

Finally, note that, given the timing (and the fact that he is away), I spoke
with George out of band, and he told me I can put his Ack on the patch. Of
course, we defer to Wei to decide if this is still acceptable.

Thanks, and sorry, and Regards, :-)
Dario
---
Dario Faggioli (1):
      xen: credit2: make runqueues be per-socket by default.

 docs/misc/xen-command-line.markdown |    2 +-
 xen/common/sched_credit2.c          |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

--
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Dario Faggioli]

Benchmarks have shown that per-socket runqueues arrangement
behaves better (e.g., we achieve better load balancing)
than the current per-core default.

Here's an example (coming from
https://lists.xen.org/archives/html/xen-devel/2016-06/msg02287.html ):

|=======================================|
| XEN BUILD TIME, LOW LOAD, NO NOISE    |
|---------------------------------------|
|       runq=core   runq=socket         |
|         35.200       33.433           |
|---------------------------------------|------------------------------|
| XEN BUILD TIME, HIGH LOAD, NO NOISE   | IPERF, HIGH LOAD, NO NOISE   |
|---------------------------------------|------------------------------|
|       runq=core   runq=socket         |     runq=core runq=socket    |
|         18.013       18.530           |       23.200     23.466      |
|---------------------------------------|------------------------------|
| XEN BUILD TIME, LOW LOAD, WITH NOISE  |
|-------------------------------------  |
|       runq=core   runq=socket         |
|         45.866       39.493           |
|---------------------------------------|------------------------------|
| XEN BUILD TIME, HIGH LOAD, WITH NOISE | IPERF, HIGH LOAD, WITH NOISE |
|---------------------------------------|------------------------------|
|       runq=core   runq=socket         |     runq=core runq=socket    |
|         36.840       29.080           |       19.967     21.000      |
|=======================================|==============================|

The only reason why we went for per-core, initially, was to
introduce some form of hyperthreading support. Now we have
hyperthreading support, independently from how runqueues
are organized (9bb9c7388 "xen: credit2: implement true SMT
support"), and thus we can switch to per-socket.

Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
---
Cc: Anshul Makkar <anshul.makkar@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
George's Ack obtained 'out of band', as he is away.
---
 docs/misc/xen-command-line.markdown |    2 +-
 xen/common/sched_credit2.c          |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 87c3023..0138978 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -527,7 +527,7 @@ The default value of `1 sec` is rather long.
 ### credit2\_runqueue
 > `= core | socket | node | all`
 
-> Default: `core`
+> Default: `socket`
 
 Specify how host CPUs are arranged in runqueues. Runqueues are kept
 balanced with respect to the load generated by the vCPUs running on
diff --git a/xen/common/sched_credit2.c b/xen/common/sched_credit2.c
index c2c563d..ef8e0d8 100644
--- a/xen/common/sched_credit2.c
+++ b/xen/common/sched_credit2.c
@@ -330,7 +330,7 @@ static const char *const opt_runqueue_str[] = {
     [OPT_RUNQUEUE_NODE] = "node",
     [OPT_RUNQUEUE_ALL] = "all"
 };
-static int __read_mostly opt_runqueue = OPT_RUNQUEUE_CORE;
+static int __read_mostly opt_runqueue = OPT_RUNQUEUE_SOCKET;
 
 static void parse_credit2_runqueue(const char *s)
 {


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Wei Liu]

On Tue, Nov 29, 2016 at 11:57:03AM +0100, Dario Faggioli wrote:
> Benchmarks have shown that per-socket runqueues arrangement
> behaves better (e.g., we achieve better load balancing)
> than the current per-core default.
> 
> Here's an example (coming from
> https://lists.xen.org/archives/html/xen-devel/2016-06/msg02287.html ):
> 
> |=======================================|
> | XEN BUILD TIME, LOW LOAD, NO NOISE    |
> |---------------------------------------|
> |       runq=core   runq=socket         |
> |         35.200       33.433           |
> |---------------------------------------|------------------------------|
> | XEN BUILD TIME, HIGH LOAD, NO NOISE   | IPERF, HIGH LOAD, NO NOISE   |
> |---------------------------------------|------------------------------|
> |       runq=core   runq=socket         |     runq=core runq=socket    |
> |         18.013       18.530           |       23.200     23.466      |
> |---------------------------------------|------------------------------|
> | XEN BUILD TIME, LOW LOAD, WITH NOISE  |
> |-------------------------------------  |
> |       runq=core   runq=socket         |
> |         45.866       39.493           |
> |---------------------------------------|------------------------------|
> | XEN BUILD TIME, HIGH LOAD, WITH NOISE | IPERF, HIGH LOAD, WITH NOISE |
> |---------------------------------------|------------------------------|
> |       runq=core   runq=socket         |     runq=core runq=socket    |
> |         36.840       29.080           |       19.967     21.000      |
> |=======================================|==============================|
> 
> The only reason why we went for per-core, initially, was to
> introduce some form of hyperthreading support. Now we have
> hyperthreading support, independently from how runqueues
> are organized (9bb9c7388 "xen: credit2: implement true SMT
> support"), and thus we can switch to per-socket.
> 
> Signed-off-by: Dario Faggioli <dario.faggioli@citrix.com>
> Acked-by: George Dunlap <george.dunlap@eu.citrix.com>
> ---
> Cc: Anshul Makkar <anshul.makkar@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
> George's Ack obtained 'out of band', as he is away.
> ---

Strictly speaking, this is not necessary for 4.8 because even though the
default configuration is not optimal, there is a simple way to change
it.

On the other hand, because credit2 is declared supported in this
release, it is better to have a sensible default setting for credit2.

The risk is that changing the default value would cause osstest to
discover bugs in the scheduler logic, hence blocking the release. But
overall I think the chance is low, but Dario can you please state to
what extend did you test this patch?

Wei.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1718 bytes --]

On Tue, 2016-11-29 at 11:14 +0000, Wei Liu wrote:
> On Tue, Nov 29, 2016 at 11:57:03AM +0100, Dario Faggioli wrote:
> Strictly speaking, this is not necessary for 4.8 because even though
> the
> default configuration is not optimal, there is a simple way to change
> it.
> 
Absolutely true.

> On the other hand, because credit2 is declared supported in this
> release, it is better to have a sensible default setting for credit2.
> 
Exactly.

> The risk is that changing the default value would cause osstest to
> discover bugs in the scheduler logic, hence blocking the release. But
> overall I think the chance is low, but Dario can you please state to
> what extend did you test this patch?
> 
I've tried to state that in the cover letter:
https://lists.xen.org/archives/html/xen-devel/2016-11/msg02616.html

In some more details, it is _months_ that I and Anshul run benchmarks
with Credit2 running in this configuration.

It's months that, all the time that I boot my test box, either to do
Credit2 related development and testing, or completely unrelated things
(e.g., recently, when testing the new Fedora 25 as HVM and PV guest), I
use Credit2 in this configuration.

George has done some runs of his schedbench suite with this
configuration (to compare Credit1 vs Credit2 per-core vs Credit2 per-
socket).

So, if you ask me "how much is this tested", my answer would be "really
really really a lot". :-)

Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Wei Liu]

On Tue, Nov 29, 2016 at 12:33:24PM +0100, Dario Faggioli wrote:
> On Tue, 2016-11-29 at 11:14 +0000, Wei Liu wrote:
> > On Tue, Nov 29, 2016 at 11:57:03AM +0100, Dario Faggioli wrote:
> > Strictly speaking, this is not necessary for 4.8 because even though
> > the
> > default configuration is not optimal, there is a simple way to change
> > it.
> > 
> Absolutely true.
> 
> > On the other hand, because credit2 is declared supported in this
> > release, it is better to have a sensible default setting for credit2.
> > 
> Exactly.
> 
> > The risk is that changing the default value would cause osstest to
> > discover bugs in the scheduler logic, hence blocking the release. But
> > overall I think the chance is low, but Dario can you please state to
> > what extend did you test this patch?
> > 
> I've tried to state that in the cover letter:
> https://lists.xen.org/archives/html/xen-devel/2016-11/msg02616.html
> 

Ah, I thought that was a dupe because it had the same subject line as
the patch, so I skipped it.

> In some more details, it is _months_ that I and Anshul run benchmarks
> with Credit2 running in this configuration.
> 
> It's months that, all the time that I boot my test box, either to do
> Credit2 related development and testing, or completely unrelated things
> (e.g., recently, when testing the new Fedora 25 as HVM and PV guest), I
> use Credit2 in this configuration.
> 
> George has done some runs of his schedbench suite with this
> configuration (to compare Credit1 vs Credit2 per-core vs Credit2 per-
> socket).
> 
> So, if you ask me "how much is this tested", my answer would be "really
> really really a lot". :-)
> 

OK. This is rather convincing to me. I will wait a bit for other people
express their opinion.

Wei.

> Dario
> -- 
> <<This happens because I choose it to happen!>> (Raistlin Majere)
> -----------------------------------------------------------------
> Dario Faggioli, Ph.D, http://about.me/dario.faggioli
> Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Jan Beulich]

>>> On 29.11.16 at 12:42, <wei.liu2@citrix.com> wrote:
> On Tue, Nov 29, 2016 at 12:33:24PM +0100, Dario Faggioli wrote:
>> On Tue, 2016-11-29 at 11:14 +0000, Wei Liu wrote:
>> > On Tue, Nov 29, 2016 at 11:57:03AM +0100, Dario Faggioli wrote:
>> > Strictly speaking, this is not necessary for 4.8 because even though
>> > the
>> > default configuration is not optimal, there is a simple way to change
>> > it.
>> > 
>> Absolutely true.
>> 
>> > On the other hand, because credit2 is declared supported in this
>> > release, it is better to have a sensible default setting for credit2.
>> > 
>> Exactly.
>> 
>> > The risk is that changing the default value would cause osstest to
>> > discover bugs in the scheduler logic, hence blocking the release. But
>> > overall I think the chance is low, but Dario can you please state to
>> > what extend did you test this patch?
>> > 
>> I've tried to state that in the cover letter:
>> https://lists.xen.org/archives/html/xen-devel/2016-11/msg02616.html 
>> 
> 
> Ah, I thought that was a dupe because it had the same subject line as
> the patch, so I skipped it.

Same here, albeit I had noticed I shouldn't have got 2 copies, so
I did look into the other one.

>> In some more details, it is _months_ that I and Anshul run benchmarks
>> with Credit2 running in this configuration.
>> 
>> It's months that, all the time that I boot my test box, either to do
>> Credit2 related development and testing, or completely unrelated things
>> (e.g., recently, when testing the new Fedora 25 as HVM and PV guest), I
>> use Credit2 in this configuration.
>> 
>> George has done some runs of his schedbench suite with this
>> configuration (to compare Credit1 vs Credit2 per-core vs Credit2 per-
>> socket).
>> 
>> So, if you ask me "how much is this tested", my answer would be "really
>> really really a lot". :-)
>> 
> 
> OK. This is rather convincing to me. I will wait a bit for other people
> express their opinion.

FWIW, I'm in favor of putting it in with the background provided.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Dario Faggioli]

[-- Attachment #1.1: Type: text/plain, Size: 1135 bytes --]

On Tue, 2016-11-29 at 04:51 -0700, Jan Beulich wrote:
> > > > On 29.11.16 at 12:42, <wei.liu2@citrix.com> wrote:
> > > I've tried to state that in the cover letter:
> > > https://lists.xen.org/archives/html/xen-devel/2016-11/msg02616.ht
> > > ml 
> > > 
> > Ah, I thought that was a dupe because it had the same subject line
> > as
> > the patch, so I skipped it.
> 
> Same here, albeit I had noticed I shouldn't have got 2 copies, so
> I did look into the other one.
> 
Ah, I see it can indeed look confusing. I hadn't thought about that,
sorry.

> > OK. This is rather convincing to me. I will wait a bit for other
> > people
> > express their opinion.
> 
> FWIW, I'm in favor of putting it in with the background provided.
> 
Whatever the outcome, thanks for having a look (well, to Jan, Wei was
kind of forced to do so! :-P :-P)

Regards,
Dario
-- 
<<This happens because I choose it to happen!>> (Raistlin Majere)
-----------------------------------------------------------------
Dario Faggioli, Ph.D, http://about.me/dario.faggioli
Senior Software Engineer, Citrix Systems R&D Ltd., Cambridge (UK)

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [for-4.8 PATCH] xen: credit2: make runqueues be per-socket by default.

[Wei Liu]

On Tue, Nov 29, 2016 at 04:51:04AM -0700, Jan Beulich wrote:
> >>> On 29.11.16 at 12:42, <wei.liu2@citrix.com> wrote:
> > On Tue, Nov 29, 2016 at 12:33:24PM +0100, Dario Faggioli wrote:
> >> On Tue, 2016-11-29 at 11:14 +0000, Wei Liu wrote:
> >> > On Tue, Nov 29, 2016 at 11:57:03AM +0100, Dario Faggioli wrote:
> >> > Strictly speaking, this is not necessary for 4.8 because even though
> >> > the
> >> > default configuration is not optimal, there is a simple way to change
> >> > it.
> >> > 
> >> Absolutely true.
> >> 
> >> > On the other hand, because credit2 is declared supported in this
> >> > release, it is better to have a sensible default setting for credit2.
> >> > 
> >> Exactly.
> >> 
> >> > The risk is that changing the default value would cause osstest to
> >> > discover bugs in the scheduler logic, hence blocking the release. But
> >> > overall I think the chance is low, but Dario can you please state to
> >> > what extend did you test this patch?
> >> > 
> >> I've tried to state that in the cover letter:
> >> https://lists.xen.org/archives/html/xen-devel/2016-11/msg02616.html 
> >> 
> > 
> > Ah, I thought that was a dupe because it had the same subject line as
> > the patch, so I skipped it.
> 
> Same here, albeit I had noticed I shouldn't have got 2 copies, so
> I did look into the other one.
> 
> >> In some more details, it is _months_ that I and Anshul run benchmarks
> >> with Credit2 running in this configuration.
> >> 
> >> It's months that, all the time that I boot my test box, either to do
> >> Credit2 related development and testing, or completely unrelated things
> >> (e.g., recently, when testing the new Fedora 25 as HVM and PV guest), I
> >> use Credit2 in this configuration.
> >> 
> >> George has done some runs of his schedbench suite with this
> >> configuration (to compare Credit1 vs Credit2 per-core vs Credit2 per-
> >> socket).
> >> 
> >> So, if you ask me "how much is this tested", my answer would be "really
> >> really really a lot". :-)
> >> 
> > 
> > OK. This is rather convincing to me. I will wait a bit for other people
> > express their opinion.
> 
> FWIW, I'm in favor of putting it in with the background provided.
> 

Release-acked-by: Wei Liu <wei.liu2@citrix.com>

Feel free to commit it with your other patches.

> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel