| Message ID | 20201222205402.2269377-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | sh: check return code of request_irq | expand |
On Wed, Dec 23, 2020 at 5:54 AM Nick Desaulniers <ndesaulniers@google.com> wrote: > > request_irq is marked __must_check, but the call in shx3_prepare_cpus > has a void return type, so it can't propagate failure to the caller. > Follow cues from hexagon and just print an error. > > Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.") > Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> > Cc: Paul Mundt <lethal@linux-sh.org> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Thanks for the patch, Nick. I just wondered if there was a better error handling than printing the message. I have no idea if the system will boot up correctly when the request_irq() fails here. I hope the maintainers will suggest something, if any. > --- > arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c > index f8a2bec0f260..1261dc7b84e8 100644 > --- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c > +++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c > @@ -73,8 +73,9 @@ static void shx3_prepare_cpus(unsigned int max_cpus) > BUILD_BUG_ON(SMP_MSG_NR >= 8); > > for (i = 0; i < SMP_MSG_NR; i++) > - request_irq(104 + i, ipi_interrupt_handler, > - IRQF_PERCPU, "IPI", (void *)(long)i); > + if (request_irq(104 + i, ipi_interrupt_handler, > + IRQF_PERCPU, "IPI", (void *)(long)i)) > + pr_err("Failed to request irq %d\n", i); > > for (i = 0; i < max_cpus; i++) > set_cpu_present(i, true); > -- > 2.29.2.729.g45daf8777d-goog >
Hi Nick! On 12/22/20 9:54 PM, Nick Desaulniers wrote: > request_irq is marked __must_check, but the call in shx3_prepare_cpus > has a void return type, so it can't propagate failure to the caller. > Follow cues from hexagon and just print an error. > > Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.") > Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> > Cc: Paul Mundt <lethal@linux-sh.org> > Reported-by: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c > index f8a2bec0f260..1261dc7b84e8 100644 > --- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c > +++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c > @@ -73,8 +73,9 @@ static void shx3_prepare_cpus(unsigned int max_cpus) > BUILD_BUG_ON(SMP_MSG_NR >= 8); > > for (i = 0; i < SMP_MSG_NR; i++) > - request_irq(104 + i, ipi_interrupt_handler, > - IRQF_PERCPU, "IPI", (void *)(long)i); > + if (request_irq(104 + i, ipi_interrupt_handler, > + IRQF_PERCPU, "IPI", (void *)(long)i)) > + pr_err("Failed to request irq %d\n", i); > > for (i = 0; i < max_cpus; i++) > set_cpu_present(i, true); > Verified on my SH-7785LCR board. Boots fine. Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
On Fri, Jan 1, 2021 at 2:50 PM John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> wrote: > > Verified on my SH-7785LCR board. Boots fine. > > Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> Thanks for testing, John! I think Masahiro was concerned about the error case (I assume you tested the happy path). In any case, if no maintainer suggests something else, looks good to me (and it is no worse than the status quo unless the `pr_err()` can somehow kill it), so: Reviewed-by: Miguel Ojeda <ojeda@kernel.org> Cheers, Miguel
Hi Miguel! On 1/1/21 9:42 PM, Miguel Ojeda wrote: > On Fri, Jan 1, 2021 at 2:50 PM John Paul Adrian Glaubitz > <glaubitz@physik.fu-berlin.de> wrote: >> >> Verified on my SH-7785LCR board. Boots fine. >> >> Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de> > > Thanks for testing, John! > > I think Masahiro was concerned about the error case (I assume you > tested the happy path). Not sure about testing the error case though. How do I make request_irq() fail? > In any case, if no maintainer suggests something else, looks good to > me (and it is no worse than the status quo unless the `pr_err()` can > somehow kill it), so: > > Reviewed-by: Miguel Ojeda <ojeda@kernel.org> I agree. Adrian
diff --git a/arch/sh/kernel/cpu/sh4a/smp-shx3.c b/arch/sh/kernel/cpu/sh4a/smp-shx3.c index f8a2bec0f260..1261dc7b84e8 100644 --- a/arch/sh/kernel/cpu/sh4a/smp-shx3.c +++ b/arch/sh/kernel/cpu/sh4a/smp-shx3.c @@ -73,8 +73,9 @@ static void shx3_prepare_cpus(unsigned int max_cpus) BUILD_BUG_ON(SMP_MSG_NR >= 8); for (i = 0; i < SMP_MSG_NR; i++) - request_irq(104 + i, ipi_interrupt_handler, - IRQF_PERCPU, "IPI", (void *)(long)i); + if (request_irq(104 + i, ipi_interrupt_handler, + IRQF_PERCPU, "IPI", (void *)(long)i)) + pr_err("Failed to request irq %d\n", i); for (i = 0; i < max_cpus; i++) set_cpu_present(i, true);
request_irq is marked __must_check, but the call in shx3_prepare_cpus has a void return type, so it can't propagate failure to the caller. Follow cues from hexagon and just print an error. Fixes: c7936b9abcf5 ("sh: smp: Hook in to the generic IPI handler for SH-X3 SMP.") Cc: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> Cc: Paul Mundt <lethal@linux-sh.org> Reported-by: Guenter Roeck <linux@roeck-us.net> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- arch/sh/kernel/cpu/sh4a/smp-shx3.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)