2015-07-17 08:06:38

by Xunlei Pang

[permalink] [raw]
Subject: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an isolate cpu

From: Xunlei Pang <[email protected]>

The sched domain is always rebuilt when hotplugging an isolate cpu,
as we can see in partition_sched_domains() with the NULL doms_new,
current sched domain is destroyed, then rebuilt. Worse still, the
rebuilding will reset all the parameters of the sched_domain. This
makes no sense and contradicts the isloate concept.

The patch avoids the rebuilding if the hotplug cpu is an isolate one.

Signed-off-by: Xunlei Pang <[email protected]>
---
kernel/sched/core.c | 34 +++++++++++++++++++++++++++++-----
1 file changed, 29 insertions(+), 5 deletions(-)

diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index bb8f3ad..bae817e 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -6862,9 +6862,12 @@ error:
}

static cpumask_var_t *doms_cur; /* current sched domains */
-static int ndoms_cur; /* number of sched domains in 'doms_cur' */
+/* effective number of sched domains in 'doms_cur' */
+static int ndoms_cur;
+/* original number of sched domains in 'doms_cur' */
+static int ndoms_orig;
+/* attribues of custom domains in 'doms_cur' */
static struct sched_domain_attr *dattr_cur;
- /* attribues of custom domains in 'doms_cur' */

/*
* Special case: If a kmalloc of a doms_cur partition (array of
@@ -6918,6 +6921,7 @@ static int init_sched_domains(const struct cpumask *cpu_map)
int err;

arch_update_cpu_topology();
+ ndoms_orig = 1;
ndoms_cur = 1;
doms_cur = alloc_sched_domains(ndoms_cur);
if (!doms_cur)
@@ -6990,6 +6994,7 @@ void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
{
int i, j, n;
int new_topology;
+ bool borrow_needed = false;

mutex_lock(&sched_domains_mutex);

@@ -6999,6 +7004,13 @@ void partition_sched_domains(int ndoms_new, cpumask_var_t doms_new[],
/* Let architecture update cpu core mappings. */
new_topology = arch_update_cpu_topology();

+ if (doms_new == NULL && doms_cur != &fallback_doms) {
+ doms_new = &fallback_doms;
+ cpumask_andnot(doms_new[0], cpu_active_mask, cpu_isolated_map);
+ borrow_needed = true;
+ WARN_ON_ONCE(dattr_new);
+ }
+
n = doms_new ? ndoms_new : 0;

/* Destroy deleted domains */
@@ -7036,10 +7048,22 @@ match2:
}

/* Remember the new sched domains */
- if (doms_cur != &fallback_doms)
- free_sched_domains(doms_cur, ndoms_cur);
+ if (doms_cur != &fallback_doms && !borrow_needed)
+ free_sched_domains(doms_cur, ndoms_orig);
kfree(dattr_cur); /* kfree(NULL) is safe */
- doms_cur = doms_new;
+
+ if (borrow_needed) {
+ /*
+ * Borrow previous doms_cur as new storage, so that
+ * fallback_doms can be used as the temporal storage
+ * in the future rebuilding.
+ */
+ cpumask_copy(doms_cur[0], doms_new[0]);
+ } else {
+ doms_cur = doms_new;
+ ndoms_orig = ndoms_new;
+ }
+
dattr_cur = dattr_new;
ndoms_cur = ndoms_new;

--
1.9.1


2015-07-30 10:30:46

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an isolate cpu

On Fri, Jul 17, 2015 at 04:03:45PM +0800, Xunlei Pang wrote:
> From: Xunlei Pang <[email protected]>
>
> The sched domain is always rebuilt when hotplugging an isolate cpu,
> as we can see in partition_sched_domains() with the NULL doms_new,
> current sched domain is destroyed, then rebuilt. Worse still, the
> rebuilding will reset all the parameters of the sched_domain. This
> makes no sense and contradicts the isloate concept.

What parameters? isolcpus should have fairly trivial domains to begin
with.

2015-07-30 11:10:53

by pang.xunlei

[permalink] [raw]
Subject: Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an isolate cpu

Hi Peter,

Peter Zijlstra <[email protected]> wrote 2015-07-30 PM 06:30:37:
> Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an
> isolate cpu
>
> On Fri, Jul 17, 2015 at 04:03:45PM +0800, Xunlei Pang wrote:
> > From: Xunlei Pang <[email protected]>
> >
> > The sched domain is always rebuilt when hotplugging an isolate cpu,
> > as we can see in partition_sched_domains() with the NULL doms_new,
> > current sched domain is destroyed, then rebuilt. Worse still, the
> > rebuilding will reset all the parameters of the sched_domain. This
> > makes no sense and contradicts the isloate concept.
>
> What parameters? isolcpus should have fairly trivial domains to begin
> with.

I mean the parameters under
/proc/sys/kernel/sched_domain/cpu<m>/domain<n>,
they will be reset to the default value once sched domain is rebuilt, if
users have tuned them before, it's very annoying.

So we should avoid the rebuilding if can, like the case in this patch.

Regards,
-Xunlei

--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s). If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited. If you have received this mail in error, please delete it and notify us immediately.

2015-07-30 11:51:51

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an isolate cpu

On Thu, Jul 30, 2015 at 06:54:47PM +0800, [email protected] wrote:
> Hi Peter,
>
> Peter Zijlstra <[email protected]> wrote 2015-07-30 PM 06:30:37:
> > Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an
> > isolate cpu
> >
> > On Fri, Jul 17, 2015 at 04:03:45PM +0800, Xunlei Pang wrote:
> > > From: Xunlei Pang <[email protected]>
> > >
> > > The sched domain is always rebuilt when hotplugging an isolate cpu,
> > > as we can see in partition_sched_domains() with the NULL doms_new,
> > > current sched domain is destroyed, then rebuilt. Worse still, the
> > > rebuilding will reset all the parameters of the sched_domain. This
> > > makes no sense and contradicts the isloate concept.
> >
> > What parameters? isolcpus should have fairly trivial domains to begin
> > with.
>
> I mean the parameters under
> /proc/sys/kernel/sched_domain/cpu<m>/domain<n>,
> they will be reset to the default value once sched domain is rebuilt, if
> users have tuned them before, it's very annoying.
>
> So we should avoid the rebuilding if can, like the case in this patch.

No, those are _DEBUG_ knobs, they're not guaranteed to have any
persistence what so ever.

2015-07-30 12:05:13

by pang.xunlei

[permalink] [raw]
Subject: Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an isolate cpu

Peter Zijlstra <[email protected]> wrote 2015-07-30 PM 07:51:39:
>
> Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an
> isolate cpu
>
> On Thu, Jul 30, 2015 at 06:54:47PM +0800, [email protected] wrote:
> > Hi Peter,
> >
> > Peter Zijlstra <[email protected]> wrote 2015-07-30 PM 06:30:37:
> > > Re: [PATCH] sched: Avoid sched domain rebuilding when hotplugging an
> > > isolate cpu
> > >
> > > On Fri, Jul 17, 2015 at 04:03:45PM +0800, Xunlei Pang wrote:
> > > > From: Xunlei Pang <[email protected]>
> > > >
> > > > The sched domain is always rebuilt when hotplugging an isolate
cpu,
> > > > as we can see in partition_sched_domains() with the NULL doms_new,
> > > > current sched domain is destroyed, then rebuilt. Worse still, the
> > > > rebuilding will reset all the parameters of the sched_domain. This
> > > > makes no sense and contradicts the isloate concept.
> > >
> > > What parameters? isolcpus should have fairly trivial domains to
begin
> > > with.
> >
> > I mean the parameters under
> > /proc/sys/kernel/sched_domain/cpu<m>/domain<n>,
> > they will be reset to the default value once sched domain is rebuilt,
if
> > users have tuned them before, it's very annoying.
> >
> > So we should avoid the rebuilding if can, like the case in this patch.
>
> No, those are _DEBUG_ knobs, they're not guaranteed to have any
> persistence what so ever.

Yea, but that's not the point of the patch, the point is the isolate cpu
is
not attached to sched domain, so when it is hotplugged, it should also not

touch the sched domain. Maybe I should remove this from the changelog :-)

Thanks,
-Xunlei

--------------------------------------------------------
ZTE Information Security Notice: The information contained in this mail (and any attachment transmitted herewith) is privileged and confidential and is intended for the exclusive use of the addressee(s). If you are not an intended recipient, any disclosure, reproduction, distribution or other dissemination or use of the information contained is strictly prohibited. If you have received this mail in error, please delete it and notify us immediately.