* [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group
@ 2021-03-04 22:29 Barry Song
2021-03-05 11:48 ` Valentin Schneider
0 siblings, 1 reply; 4+ messages in thread
From: Barry Song @ 2021-03-04 22:29 UTC (permalink / raw)
To: valentin.schneider, vincent.guittot, mingo, peterz, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman
Cc: linux-kernel, linuxarm, Barry Song
mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so
it must be a subset of sched_group_span(sg). Though cpumask_first_and
doesn't lead to a wrong result of balance cpu, it is pointless to do
cpumask_and again.
Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
---
kernel/sched/topology.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 12f8058..45f3db2 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -934,7 +934,7 @@ static void init_overlap_sched_group(struct sched_domain *sd,
int cpu;
build_balance_mask(sd, sg, mask);
- cpu = cpumask_first_and(sched_group_span(sg), mask);
+ cpu = cpumask_first(mask);
sg->sgc = *per_cpu_ptr(sdd->sgc, cpu);
if (atomic_inc_return(&sg->sgc->ref) == 1)
--
1.8.3.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group
2021-03-04 22:29 [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group Barry Song
@ 2021-03-05 11:48 ` Valentin Schneider
2021-03-05 20:25 ` Song Bao Hua (Barry Song)
0 siblings, 1 reply; 4+ messages in thread
From: Valentin Schneider @ 2021-03-05 11:48 UTC (permalink / raw)
To: Barry Song, vincent.guittot, mingo, peterz, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman
Cc: linux-kernel, linuxarm, Barry Song
On 05/03/21 11:29, Barry Song wrote:
> mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so
> it must be a subset of sched_group_span(sg).
So we should indeed have
cpumask_subset(sched_group_span(sg), mask)
but that doesn't imply
cpumask_first(sched_group_span(sg)) == cpumask_first(mask)
does it? I'm thinking if in your topology of N CPUs, CPUs 0 and N-1 are the
furthest away, you will most likely hit
!cpumask_equal(sg_pan, sched_domain_span(sibling->child))
^^^^^^ ^^^^^^^^^^^^^
CPUN-1 CPU0
which should be the case on your Kunpeng920 system.
> Though cpumask_first_and
> doesn't lead to a wrong result of balance cpu, it is pointless to do
> cpumask_and again.
>
> Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> ---
> kernel/sched/topology.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> index 12f8058..45f3db2 100644
> --- a/kernel/sched/topology.c
> +++ b/kernel/sched/topology.c
> @@ -934,7 +934,7 @@ static void init_overlap_sched_group(struct sched_domain *sd,
> int cpu;
>
> build_balance_mask(sd, sg, mask);
> - cpu = cpumask_first_and(sched_group_span(sg), mask);
> + cpu = cpumask_first(mask);
>
> sg->sgc = *per_cpu_ptr(sdd->sgc, cpu);
> if (atomic_inc_return(&sg->sgc->ref) == 1)
> --
> 1.8.3.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group
2021-03-05 11:48 ` Valentin Schneider
@ 2021-03-05 20:25 ` Song Bao Hua (Barry Song)
2021-03-07 18:19 ` Valentin Schneider
0 siblings, 1 reply; 4+ messages in thread
From: Song Bao Hua (Barry Song) @ 2021-03-05 20:25 UTC (permalink / raw)
To: Valentin Schneider, vincent.guittot, mingo, peterz, juri.lelli,
dietmar.eggemann, rostedt, bsegall, mgorman
Cc: linux-kernel, linuxarm
> -----Original Message-----
> From: Valentin Schneider [mailto:valentin.schneider@arm.com]
> Sent: Saturday, March 6, 2021 12:49 AM
> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
> vincent.guittot@linaro.org; mingo@redhat.com; peterz@infradead.org;
> juri.lelli@redhat.com; dietmar.eggemann@arm.com; rostedt@goodmis.org;
> bsegall@google.com; mgorman@suse.de
> Cc: linux-kernel@vger.kernel.org; linuxarm@openeuler.org; Song Bao Hua (Barry
> Song) <song.bao.hua@hisilicon.com>
> Subject: Re: [PATCH] sched/topology: remove redundant cpumask_and in
> init_overlap_sched_group
>
> On 05/03/21 11:29, Barry Song wrote:
> > mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so
> > it must be a subset of sched_group_span(sg).
>
> So we should indeed have
>
> cpumask_subset(sched_group_span(sg), mask)
>
> but that doesn't imply
>
> cpumask_first(sched_group_span(sg)) == cpumask_first(mask)
>
> does it? I'm thinking if in your topology of N CPUs, CPUs 0 and N-1 are the
> furthest away, you will most likely hit
It is true:
cpumask_first(sched_group_span(sg)) != cpumask_first(mask)
but
cpumask_first_and(sched_group_span(sg), mask) = cpumask_first(mask)
since mask is always subset of sched_group_span(sg).
/**
* cpumask_first_and - return the first cpu from *srcp1 & *srcp2
* @src1p: the first input
* @src2p: the second input
*
* Returns >= nr_cpu_ids if no cpus set in both. See also cpumask_next_and().
*/
*srcp2 is subset of *srcp1, so *srcp1 & *srcp2 = *srcp2
>
> !cpumask_equal(sg_pan, sched_domain_span(sibling->child))
> ^^^^^^ ^^^^^^^^^^^^^
> CPUN-1 CPU0
>
> which should be the case on your Kunpeng920 system.
>
> > Though cpumask_first_and
> > doesn't lead to a wrong result of balance cpu, it is pointless to do
> > cpumask_and again.
> >
> > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com>
> > ---
> > kernel/sched/topology.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
> > index 12f8058..45f3db2 100644
> > --- a/kernel/sched/topology.c
> > +++ b/kernel/sched/topology.c
> > @@ -934,7 +934,7 @@ static void init_overlap_sched_group(struct sched_domain
> *sd,
> > int cpu;
> >
> > build_balance_mask(sd, sg, mask);
> > - cpu = cpumask_first_and(sched_group_span(sg), mask);
> > + cpu = cpumask_first(mask);
> >
> > sg->sgc = *per_cpu_ptr(sdd->sgc, cpu);
> > if (atomic_inc_return(&sg->sgc->ref) == 1)
> > --
> > 1.8.3.1
Thanks
Barry
^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group
2021-03-05 20:25 ` Song Bao Hua (Barry Song)
@ 2021-03-07 18:19 ` Valentin Schneider
0 siblings, 0 replies; 4+ messages in thread
From: Valentin Schneider @ 2021-03-07 18:19 UTC (permalink / raw)
To: Song Bao Hua (Barry Song),
vincent.guittot, mingo, peterz, juri.lelli, dietmar.eggemann,
rostedt, bsegall, mgorman
Cc: linux-kernel, linuxarm
On 05/03/21 20:25, Song Bao Hua (Barry Song) wrote:
>> -----Original Message-----
>> From: Valentin Schneider [mailto:valentin.schneider@arm.com]
>> Sent: Saturday, March 6, 2021 12:49 AM
>> To: Song Bao Hua (Barry Song) <song.bao.hua@hisilicon.com>;
>> vincent.guittot@linaro.org; mingo@redhat.com; peterz@infradead.org;
>> juri.lelli@redhat.com; dietmar.eggemann@arm.com; rostedt@goodmis.org;
>> bsegall@google.com; mgorman@suse.de
>> Cc: linux-kernel@vger.kernel.org; linuxarm@openeuler.org; Song Bao Hua (Barry
>> Song) <song.bao.hua@hisilicon.com>
>> Subject: Re: [PATCH] sched/topology: remove redundant cpumask_and in
>> init_overlap_sched_group
>>
>> On 05/03/21 11:29, Barry Song wrote:
>> > mask is built in build_balance_mask() by for_each_cpu(i, sg_span), so
>> > it must be a subset of sched_group_span(sg).
>>
>> So we should indeed have
>>
>> cpumask_subset(sched_group_span(sg), mask)
>>
>> but that doesn't imply
>>
>> cpumask_first(sched_group_span(sg)) == cpumask_first(mask)
>>
>> does it? I'm thinking if in your topology of N CPUs, CPUs 0 and N-1 are the
>> furthest away, you will most likely hit
>
> It is true:
> cpumask_first(sched_group_span(sg)) != cpumask_first(mask)
>
> but
>
> cpumask_first_and(sched_group_span(sg), mask) = cpumask_first(mask)
>
> since mask is always subset of sched_group_span(sg).
>
You're right, I read it the wrong way around, sorry about that.
Reviewed-by: Valentin Schneider <Valentin.Schneider@arm.com>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-07 18:20 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 22:29 [PATCH] sched/topology: remove redundant cpumask_and in init_overlap_sched_group Barry Song
2021-03-05 11:48 ` Valentin Schneider
2021-03-05 20:25 ` Song Bao Hua (Barry Song)
2021-03-07 18:19 ` Valentin Schneider
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.