| Message ID | 20240625082736.7238-1-jgross@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | xen/sched: fix rare error case in cpu_schedule_down() | expand |
On 25.06.2024 10:27, Juergen Gross wrote: > In case cpu_schedule_up() is failing to allocate memory for struct > sched_resource, Or (just to mention it again) in case the function isn't called at all because some earlier CPU_UP_PREPARE handler fails. > cpu_schedule_down() will be called with the > sched_resource pointer being NULL. This needs to be handled. > > Reported-by: Jan Beulich <jbeulich@suse.com> > Fixes: 207589dbacd4 ("xen/sched: move per cpu scheduler private data into struct sched_resource") > Signed-off-by: Juergen Gross <jgross@suse.com> Reviewed-by: Jan Beulich <jbeulich@suse.com> You didn't tag it for 4.19 and you also didn't Cc Oleksii, so I expect you deem this minor enough to be delayed until 4.20 opens, despite the Fixes: tag? Jan
On 25.06.24 10:33, Jan Beulich wrote: > On 25.06.2024 10:27, Juergen Gross wrote: >> In case cpu_schedule_up() is failing to allocate memory for struct >> sched_resource, > > Or (just to mention it again) in case the function isn't called at all > because some earlier CPU_UP_PREPARE handler fails. > >> cpu_schedule_down() will be called with the >> sched_resource pointer being NULL. This needs to be handled. >> >> Reported-by: Jan Beulich <jbeulich@suse.com> >> Fixes: 207589dbacd4 ("xen/sched: move per cpu scheduler private data into struct sched_resource") >> Signed-off-by: Juergen Gross <jgross@suse.com> > > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > You didn't tag it for 4.19 and you also didn't Cc Oleksii, so I expect you > deem this minor enough to be delayed until 4.20 opens, despite the Fixes: > tag? Correct. Juergen
On 25.06.24 10:33, Jan Beulich wrote: > On 25.06.2024 10:27, Juergen Gross wrote: >> In case cpu_schedule_up() is failing to allocate memory for struct >> sched_resource, > > Or (just to mention it again) in case the function isn't called at all > because some earlier CPU_UP_PREPARE handler fails. This remark made me looking into the notifier implementation. I think this patch should be reworked significantly: In cpu_up() the CPU_UP_CANCELED notifier call is using the same notifier_block as the one used by CPU_UP_PREPARE before. This means, that only the handlers which didn't fail for CPU_UP_PREPARE will be called with CPU_UP_CANCELED. So there is no such case as "in case the function isn't called at all because some earlier CPU_UP_PREPARE handler fails". And a failure of cpu_schedule_up() needs to undo all externally visible modifications instead of hoping that the CPU_UP_CANCELED notifier will do that. So: V2 of the patch will be needed. Juergen
diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c index d84b65f197..0dc86b8f6c 100644 --- a/xen/common/sched/core.c +++ b/xen/common/sched/core.c @@ -2829,6 +2829,8 @@ static void cpu_schedule_down(unsigned int cpu) rcu_read_lock(&sched_res_rculock); sr = get_sched_res(cpu); + if ( !sr ) + goto out; kill_timer(&sr->s_timer); @@ -2839,6 +2841,7 @@ static void cpu_schedule_down(unsigned int cpu) sr->sched_unit_idle = NULL; call_rcu(&sr->rcu, sched_res_free); + out: rcu_read_unlock(&sched_res_rculock); }
In case cpu_schedule_up() is failing to allocate memory for struct sched_resource, cpu_schedule_down() will be called with the sched_resource pointer being NULL. This needs to be handled. Reported-by: Jan Beulich <jbeulich@suse.com> Fixes: 207589dbacd4 ("xen/sched: move per cpu scheduler private data into struct sched_resource") Signed-off-by: Juergen Gross <jgross@suse.com> --- xen/common/sched/core.c | 3 +++ 1 file changed, 3 insertions(+)