Messages in this thread Patch in this message | | | Date | Wed, 4 Dec 2019 11:09:25 +0100 | From | Vincent Guittot <> | Subject | Re: Null pointer crash at find_idlest_group on db845c w/ linus/master |
| |
Le Wednesday 04 Dec 2019 à 09:42:17 (+0000), Qais Yousef a écrit : > On 12/04/19 09:06, Vincent Guittot wrote: > > Hi John, > > > > On Tue, 3 Dec 2019 at 20:15, John Stultz <john.stultz@linaro.org> wrote: > > > > > > With today's linus/master on db845c running android, I'm able to > > > fairly easily reproduce the following crash. I've not had a chance to > > > bisect it yet, but I'm suspecting its connected to Vincent's recent > > > rework. > > > > Does the crash happen randomly or after a specific action ? > > I have a db845 so I can try to reproduce it locally. > > Isn't there a chance we use local_sgs without initializing it in that function?
Normally not because the cpu belongs to its sched_domain
Now, we test that a group has at least one allowed CPU for the task so we could skip the local group with the correct/wrong p->cpus_ptr
The path is used for fork/exec ibut also for wakeup path for b.L when the task doesn't fit in the CPUs
So we can probably imagine a scenario where we change task affinity while sleeping. If the wakeup happens on a CPU that belongs to the group that is not allowed, we can imagine that we skip the local_group
John,
Could you try the fix below ?
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c index 08a233e..bcd216d 100644 --- a/kernel/sched/fair.c +++ b/kernel/sched/fair.c @@ -8417,6 +8417,10 @@ find_idlest_group(struct sched_domain *sd, struct task_struct *p, if (!idlest) return NULL; + /* The local group has been skipped because of cpu affinity */ + if (!local) + return idlest; + /* * If the local group is idler than the selected idlest group * don't try and push the task.
> > AFAICS we define local_sgs on the stack but not always could be populated with > the right values. I can't see tmp_sgs being used in the function too. Could > this cause the/a problem? > > 8377 struct sg_lb_stats local_sgs, tmp_sgs; > . > . > . > 8399 if (local_group) { > 8400 sgs = &local_sgs; > 8401 local = group; > 8402 } else { > 8403 sgs = &tmp_sgs; > 8404 } > 8405 > 8406 update_sg_wakeup_stats(sd, group, sgs, p); > > Cheers > > -- > Qais Youef
| |