Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1281700 > unrolled thread
| Started by | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| First post | 2015-12-02 13:00 +0100 |
| Last post | 2015-12-04 13:00 +0100 |
| Articles | 14 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Xunlei Pang <xlpang@redhat.com> - 2015-12-02 13:00 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Peter Zijlstra <peterz@infradead.org> - 2015-12-02 13:40 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Xunlei Pang <xlpang@redhat.com> - 2015-12-02 14:20 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Peter Zijlstra <peterz@infradead.org> - 2015-12-02 17:30 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Xunlei Pang <xlpang@redhat.com> - 2015-12-03 03:50 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Ingo Molnar <mingo@kernel.org> - 2015-12-03 09:30 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Xunlei Pang <xlpang@redhat.com> - 2015-12-03 13:00 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Ingo Molnar <mingo@kernel.org> - 2015-12-04 09:10 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Peter Zijlstra <peterz@infradead.org> - 2015-12-04 09:30 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Ingo Molnar <mingo@kernel.org> - 2015-12-04 09:40 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Xunlei Pang <xlpang@redhat.com> - 2015-12-04 09:30 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Ingo Molnar <mingo@kernel.org> - 2015-12-04 09:40 +0100
Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() Peter Zijlstra <peterz@infradead.org> - 2015-12-03 10:40 +0100
[tip:locking/core] sched/core: Clear the root_domain cpumasks in init_rootdomain() tip-bot for Xunlei Pang <tipbot@zytor.com> - 2015-12-04 13:00 +0100
| From | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| Date | 2015-12-02 13:00 +0100 |
| Subject | [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBeeK-6GK-13@gated-at.bofh.it> |
root_domain::rto_mask allocated through alloc_cpumask_var()
contains garbage data, this may cause problems. For instance,
When doing pull_rt_task(), it may do useless iterations if
rto_mask retains some extra garbage bits. Worse still, this
violates the isolated domain rule for clustered scheduling
using cpuset, because the tasks(with all the cpus allowed)
belongs to one root domain can be pulled away into another
root domain.
The patch cleans the garbage by using zalloc_cpumask_var()
instead of alloc_cpumask_var() for root_domain::rto_mask
allocation, thereby addressing the issues.
Do the same thing for root_domain's other cpumask memembers:
dlo_mask, span, and online.
Signed-off-by: Xunlei Pang <xlpang@redhat.com>
---
kernel/sched/core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 5b420d2..5691953 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5858,13 +5858,13 @@ static int init_rootdomain(struct root_domain *rd)
{
memset(rd, 0, sizeof(*rd));
- if (!alloc_cpumask_var(&rd->span, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->span, GFP_KERNEL))
goto out;
- if (!alloc_cpumask_var(&rd->online, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->online, GFP_KERNEL))
goto free_span;
- if (!alloc_cpumask_var(&rd->dlo_mask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->dlo_mask, GFP_KERNEL))
goto free_online;
- if (!alloc_cpumask_var(&rd->rto_mask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->rto_mask, GFP_KERNEL))
goto free_dlo_mask;
init_dl_bw(&rd->dl_bw);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-12-02 13:40 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBeRs-79R-19@gated-at.bofh.it> |
| In reply to | #1281700 |
On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote: > The patch cleans the garbage by using zalloc_cpumask_var() > instead of alloc_cpumask_var() for root_domain::rto_mask > allocation, thereby addressing the issues. How did you notice this? Also do we want to do the same for the kmalloc in alloc_rootdomain() ? -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| Date | 2015-12-02 14:20 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBfu9-7Ey-1@gated-at.bofh.it> |
| In reply to | #1281744 |
Hi Peter, On 12/02/2015 at 08:34 PM, Peter Zijlstra wrote: > On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote: >> The patch cleans the garbage by using zalloc_cpumask_var() >> instead of alloc_cpumask_var() for root_domain::rto_mask >> allocation, thereby addressing the issues. > How did you notice this? Also do we want to do the same for the kmalloc When doing review. > in alloc_rootdomain() ? There is a "memset(rd, 0, sizeof(*rd))" in init_rootdomain(), so I don't think we need to do this in alloc_rootdomain(). Regards, Xunlei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-12-02 17:30 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBis3-19N-29@gated-at.bofh.it> |
| In reply to | #1281772 |
On Wed, Dec 02, 2015 at 09:12:30PM +0800, Xunlei Pang wrote: > Hi Peter, > > On 12/02/2015 at 08:34 PM, Peter Zijlstra wrote: > > On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote: > >> The patch cleans the garbage by using zalloc_cpumask_var() > >> instead of alloc_cpumask_var() for root_domain::rto_mask > >> allocation, thereby addressing the issues. > > How did you notice this? Also do we want to do the same for the kmalloc > > When doing review. Nice, will you be looking for similar issues elsewhere in the scheduler too? > > in alloc_rootdomain() ? > > There is a "memset(rd, 0, sizeof(*rd))" in init_rootdomain(), > so I don't think we need to do this in alloc_rootdomain(). Ah, right there is. Which also clears the mask for small systems. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| Date | 2015-12-03 03:50 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBs81-7iq-7@gated-at.bofh.it> |
| In reply to | #1281982 |
Hi Peter, On 12/03/2015 at 12:25 AM, Peter Zijlstra wrote: > On Wed, Dec 02, 2015 at 09:12:30PM +0800, Xunlei Pang wrote: >> Hi Peter, >> >> On 12/02/2015 at 08:34 PM, Peter Zijlstra wrote: >>> On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote: >>>> The patch cleans the garbage by using zalloc_cpumask_var() >>>> instead of alloc_cpumask_var() for root_domain::rto_mask >>>> allocation, thereby addressing the issues. >>> How did you notice this? Also do we want to do the same for the kmalloc >> When doing review. > Nice, will you be looking for similar issues elsewhere in the scheduler > too? Sure :-) >>> in alloc_rootdomain() ? >> There is a "memset(rd, 0, sizeof(*rd))" in init_rootdomain(), >> so I don't think we need to do this in alloc_rootdomain(). > Ah, right there is. Which also clears the mask for small systems. Yeah, maybe we can improve it using alloc_cpumask_var() with __GFP_ZERO instead of zalloc_cpumask_var() to avoid duplicate clean for small systems. Regards, Xunlei -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-12-03 09:30 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBxr4-2rt-21@gated-at.bofh.it> |
| In reply to | #1282655 |
* Xunlei Pang <xlpang@redhat.com> wrote:
> Hi Peter,
>
> On 12/03/2015 at 12:25 AM, Peter Zijlstra wrote:
> > On Wed, Dec 02, 2015 at 09:12:30PM +0800, Xunlei Pang wrote:
> >> Hi Peter,
> >>
> >> On 12/02/2015 at 08:34 PM, Peter Zijlstra wrote:
> >>> On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote:
> >>>> The patch cleans the garbage by using zalloc_cpumask_var()
> >>>> instead of alloc_cpumask_var() for root_domain::rto_mask
> >>>> allocation, thereby addressing the issues.
> >>> How did you notice this? Also do we want to do the same for the kmalloc
> >> When doing review.
> > Nice, will you be looking for similar issues elsewhere in the scheduler
> > too?
>
> Sure :-)
Hm, is the alloc_cpumask_var() done in alloc_sched_domains() safe?
At least the usage pattern in init_sched_domains() looks unsafe:
doms_cur = alloc_sched_domains(ndoms_cur);
if (!doms_cur)
doms_cur = &fallback_doms;
cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
I think alloc_cpumask_var() is a fundamentally unsafe or at least fragile
operation, because the uninitialized variable bug will only happen on large CPU
count kernels AFAICS - so it's inviting such bugs.
How about we rename alloc_cpumask_var() to alloc_cpumask_var_noinit() or at least
__alloc_cpumask_var(), to make this property easier to see?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| Date | 2015-12-03 13:00 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBAIi-4rp-21@gated-at.bofh.it> |
| In reply to | #1282782 |
Hi Ingo, On 12/03/2015 at 04:28 PM, Ingo Molnar wrote: > * Xunlei Pang <xlpang@redhat.com> wrote: > >> Hi Peter, >> >> On 12/03/2015 at 12:25 AM, Peter Zijlstra wrote: >>> On Wed, Dec 02, 2015 at 09:12:30PM +0800, Xunlei Pang wrote: >>>> Hi Peter, >>>> >>>> On 12/02/2015 at 08:34 PM, Peter Zijlstra wrote: >>>>> On Wed, Dec 02, 2015 at 07:52:59PM +0800, Xunlei Pang wrote: >>>>>> The patch cleans the garbage by using zalloc_cpumask_var() >>>>>> instead of alloc_cpumask_var() for root_domain::rto_mask >>>>>> allocation, thereby addressing the issues. >>>>> How did you notice this? Also do we want to do the same for the kmalloc >>>> When doing review. >>> Nice, will you be looking for similar issues elsewhere in the scheduler >>> too? >> Sure :-) > Hm, is the alloc_cpumask_var() done in alloc_sched_domains() safe? Until now, I haven't found any other similar issues, but I will check further. > > At least the usage pattern in init_sched_domains() looks unsafe: > > doms_cur = alloc_sched_domains(ndoms_cur); > if (!doms_cur) > doms_cur = &fallback_doms; > cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map); > > I think alloc_cpumask_var() is a fundamentally unsafe or at least fragile > operation, because the uninitialized variable bug will only happen on large CPU > count kernels AFAICS - so it's inviting such bugs. > > How about we rename alloc_cpumask_var() to alloc_cpumask_var_noinit() or at least > __alloc_cpumask_var(), to make this property easier to see? There have already been many call sites of it in the kernel, at least we still have zalloc_cpumask_var(), maybe we could add some function comments, reminding people of thinking of zalloc_cpumask_var() for their cases. Regards, Xunlei > > Thanks, > > Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-12-04 09:10 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBTBf-7g-9@gated-at.bofh.it> |
| In reply to | #1282975 |
* Xunlei Pang <xlpang@redhat.com> wrote:
> > Hm, is the alloc_cpumask_var() done in alloc_sched_domains() safe?
>
> Until now, I haven't found any other similar issues, but I will check further.
>
> >
> > At least the usage pattern in init_sched_domains() looks unsafe:
> >
> > doms_cur = alloc_sched_domains(ndoms_cur);
> > if (!doms_cur)
> > doms_cur = &fallback_doms;
> > cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
So is this pattern in init_sched_domains() correct, for OFFSTACK=y?
It looks wrong to me, as alloc_sched_domains() allocates an uninitialized cpumask
via alloc_cpumask_var() and returns it:
cpumask_var_t *alloc_sched_domains(unsigned int ndoms)
{
int i;
cpumask_var_t *doms;
doms = kmalloc(sizeof(*doms) * ndoms, GFP_KERNEL);
if (!doms)
return NULL;
for (i = 0; i < ndoms; i++) {
if (!alloc_cpumask_var(&doms[i], GFP_KERNEL)) {
free_sched_domains(doms, i);
return NULL;
}
}
return doms;
}
and then this code:
> > cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
uses it without first clearing it.
So is this another such bug, or am I missing something?
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-12-04 09:30 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBTUD-dS-11@gated-at.bofh.it> |
| In reply to | #1283619 |
On Fri, Dec 04, 2015 at 09:09:01AM +0100, Ingo Molnar wrote: > and then this code: > > > > cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map); > > uses it without first clearing it. > > So is this another such bug, or am I missing something? It uses it as destination, it does a complete write of the mask. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-12-04 09:40 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBU4k-h7-41@gated-at.bofh.it> |
| In reply to | #1283630 |
* Peter Zijlstra <peterz@infradead.org> wrote: > On Fri, Dec 04, 2015 at 09:09:01AM +0100, Ingo Molnar wrote: > > and then this code: > > > > > > cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map); > > > > uses it without first clearing it. > > > > So is this another such bug, or am I missing something? > > It uses it as destination, it does a complete write of the mask. ah, indeed! The cpumask primitive confused me. Thanks, Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Xunlei Pang <xlpang@redhat.com> |
|---|---|
| Date | 2015-12-04 09:30 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBTUD-dS-13@gated-at.bofh.it> |
| In reply to | #1283619 |
Hi Ingo,
On 12/04/2015 at 04:09 PM, Ingo Molnar wrote:
> * Xunlei Pang <xlpang@redhat.com> wrote:
>
>>> Hm, is the alloc_cpumask_var() done in alloc_sched_domains() safe?
>> Until now, I haven't found any other similar issues, but I will check further.
>>
>>> At least the usage pattern in init_sched_domains() looks unsafe:
>>>
>>> doms_cur = alloc_sched_domains(ndoms_cur);
>>> if (!doms_cur)
>>> doms_cur = &fallback_doms;
>>> cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
> So is this pattern in init_sched_domains() correct, for OFFSTACK=y?
>
> It looks wrong to me, as alloc_sched_domains() allocates an uninitialized cpumask
> via alloc_cpumask_var() and returns it:
>
> cpumask_var_t *alloc_sched_domains(unsigned int ndoms)
> {
> int i;
> cpumask_var_t *doms;
>
> doms = kmalloc(sizeof(*doms) * ndoms, GFP_KERNEL);
> if (!doms)
> return NULL;
> for (i = 0; i < ndoms; i++) {
> if (!alloc_cpumask_var(&doms[i], GFP_KERNEL)) {
> free_sched_domains(doms, i);
> return NULL;
> }
> }
> return doms;
> }
>
> and then this code:
>
>>> cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
> uses it without first clearing it.
>
> So is this another such bug, or am I missing something?
Yeah, I noticed that as well. But fortunately cpumask_andnot(),
cpumask_and() and the like will clear doms_cur[] indirectly, also
cpu_isolated_map, cpu_active_mask, etc doesn't contain any
garbage bits. I also checked the use of it by cpuset, no extra such
bug found by me so far.
Regards,
Xunlei
>
> Thanks,
>
> Ingo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Ingo Molnar <mingo@kernel.org> |
|---|---|
| Date | 2015-12-04 09:40 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBU4i-h7-3@gated-at.bofh.it> |
| In reply to | #1283631 |
* Xunlei Pang <xlpang@redhat.com> wrote:
> Hi Ingo,
>
> On 12/04/2015 at 04:09 PM, Ingo Molnar wrote:
> > * Xunlei Pang <xlpang@redhat.com> wrote:
> >
> >>> Hm, is the alloc_cpumask_var() done in alloc_sched_domains() safe?
> >> Until now, I haven't found any other similar issues, but I will check further.
> >>
> >>> At least the usage pattern in init_sched_domains() looks unsafe:
> >>>
> >>> doms_cur = alloc_sched_domains(ndoms_cur);
> >>> if (!doms_cur)
> >>> doms_cur = &fallback_doms;
> >>> cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
> > So is this pattern in init_sched_domains() correct, for OFFSTACK=y?
> >
> > It looks wrong to me, as alloc_sched_domains() allocates an uninitialized cpumask
> > via alloc_cpumask_var() and returns it:
> >
> > cpumask_var_t *alloc_sched_domains(unsigned int ndoms)
> > {
> > int i;
> > cpumask_var_t *doms;
> >
> > doms = kmalloc(sizeof(*doms) * ndoms, GFP_KERNEL);
> > if (!doms)
> > return NULL;
> > for (i = 0; i < ndoms; i++) {
> > if (!alloc_cpumask_var(&doms[i], GFP_KERNEL)) {
> > free_sched_domains(doms, i);
> > return NULL;
> > }
> > }
> > return doms;
> > }
> >
> > and then this code:
> >
> >>> cpumask_andnot(doms_cur[0], cpu_map, cpu_isolated_map);
> > uses it without first clearing it.
> >
> > So is this another such bug, or am I missing something?
>
> Yeah, I noticed that as well. But fortunately cpumask_andnot(),
> cpumask_and() and the like will clear doms_cur[] indirectly, also
> cpu_isolated_map, cpu_active_mask, etc doesn't contain any
> garbage bits. I also checked the use of it by cpuset, no extra such
> bug found by me so far.
Great, thanks for double checking!
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2015-12-03 10:40 +0100 |
| Subject | Re: [PATCH] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBywN-38I-13@gated-at.bofh.it> |
| In reply to | #1282655 |
On Thu, Dec 03, 2015 at 10:44:08AM +0800, Xunlei Pang wrote: > > Nice, will you be looking for similar issues elsewhere in the scheduler > > too? > > Sure :-) Thanks! > >>> in alloc_rootdomain() ? > >> There is a "memset(rd, 0, sizeof(*rd))" in init_rootdomain(), > >> so I don't think we need to do this in alloc_rootdomain(). > > Ah, right there is. Which also clears the mask for small systems. > > Yeah, maybe we can improve it using alloc_cpumask_var() with > __GFP_ZERO instead of zalloc_cpumask_var() to avoid duplicate > clean for small systems. This isn't a performance critical path, so clarity and correctness are more important than performance. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Xunlei Pang <tipbot@zytor.com> |
|---|---|
| Date | 2015-12-04 13:00 +0100 |
| Subject | [tip:locking/core] sched/core: Clear the root_domain cpumasks in init_rootdomain() |
| Message-ID | <qBXbU-2b5-73@gated-at.bofh.it> |
| In reply to | #1281700 |
Commit-ID: 8295c69925ad53ec32ca54ac9fc194ff21bc40e2
Gitweb: http://git.kernel.org/tip/8295c69925ad53ec32ca54ac9fc194ff21bc40e2
Author: Xunlei Pang <xlpang@redhat.com>
AuthorDate: Wed, 2 Dec 2015 19:52:59 +0800
Committer: Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 4 Dec 2015 10:16:21 +0100
sched/core: Clear the root_domain cpumasks in init_rootdomain()
root_domain::rto_mask allocated through alloc_cpumask_var()
contains garbage data, this may cause problems. For instance,
When doing pull_rt_task(), it may do useless iterations if
rto_mask retains some extra garbage bits. Worse still, this
violates the isolated domain rule for clustered scheduling
using cpuset, because the tasks(with all the cpus allowed)
belongs to one root domain can be pulled away into another
root domain.
The patch cleans the garbage by using zalloc_cpumask_var()
instead of alloc_cpumask_var() for root_domain::rto_mask
allocation, thereby addressing the issues.
Do the same thing for root_domain's other cpumask memembers:
dlo_mask, span, and online.
Signed-off-by: Xunlei Pang <xlpang@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <stable@vger.kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/1449057179-29321-1-git-send-email-xlpang@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
kernel/sched/core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index fc8c987..eee4ee6 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -5846,13 +5846,13 @@ static int init_rootdomain(struct root_domain *rd)
{
memset(rd, 0, sizeof(*rd));
- if (!alloc_cpumask_var(&rd->span, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->span, GFP_KERNEL))
goto out;
- if (!alloc_cpumask_var(&rd->online, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->online, GFP_KERNEL))
goto free_span;
- if (!alloc_cpumask_var(&rd->dlo_mask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->dlo_mask, GFP_KERNEL))
goto free_online;
- if (!alloc_cpumask_var(&rd->rto_mask, GFP_KERNEL))
+ if (!zalloc_cpumask_var(&rd->rto_mask, GFP_KERNEL))
goto free_dlo_mask;
init_dl_bw(&rd->dl_bw);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web