Messages in this thread | | | From | Valentin Schneider <> | Subject | Re: [PATCH v4] cpu/hotplug: Do not bail-out in DYING/STARTING sections | Date | Fri, 22 Jul 2022 19:35:34 +0100 |
| |
On 19/07/22 16:48, Vincent Donnefort wrote: > On Tue, Jul 19, 2022 at 04:12:03PM +0100, Valentin Schneider wrote: >> On 04/07/22 14:13, Vincent Donnefort wrote: >> > +static int _cpuhp_invoke_callback_range(bool bringup, >> > + unsigned int cpu, >> > + struct cpuhp_cpu_state *st, >> > + enum cpuhp_state target, >> > + bool nofail) >> [...] >> > + if (nofail) { >> > + pr_warn("CPU %u %s state %s (%d) failed (%d)\n", >> > + cpu, bringup ? "UP" : "DOWN", >> > + cpuhp_get_step(st->state)->name, >> > + st->state, err); >> > + ret = -1; >> >> On a single failure we'll get two warns (WARN_ON_ONCE() + pr_warn(), and >> then subsequently just the pr_warn()), is that intended? > > It does, this is to keep the backtrace that used to be here... but now, giving > a second thought, we can probably get rid of it and just keep the pr_warn()? > >> >> Also, why not have ret = err here? > > If two states fail, the ret wouldn't mean much, hence a default "-1" just for > the WARN_ONCE.
Right
> But if we drop the latter, that would simplify the problem of > knowing which error code to return. >
We need to drop one of the two, the pr_warn() will probably be more useful if/when we need to debug this, so go for it.
| |