| Message ID | 20161018160719.31252-1-punit.agrawal@arm.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Rafael Wysocki |
| Headers | show |
On 10/18/2016 10:07 AM, Punit Agrawal wrote: > Although ghes_proc tests for error while reading the error status, it > always return success (0). Fix this by propagating the return value. > > Fixes: d334a49113a4a33 ("ACPI, APEI, Generic Hardware Error Source memory error support") > Signed-of-by: Punit Agrawal <punit.agrawa.@arm.com> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: Len Brown <lenb@kernel.org> Tested-by: Tyler Baicar <tbaicar@codeaurora.org> Thanks, Tyler > --- > drivers/acpi/apei/ghes.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index f0a029e..0d099a2 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -662,7 +662,7 @@ static int ghes_proc(struct ghes *ghes) > ghes_do_proc(ghes, ghes->estatus); > out: > ghes_clear_estatus(ghes); > - return 0; > + return rc; > } > > static void ghes_add_timer(struct ghes *ghes)
"Baicar, Tyler" <tbaicar@codeaurora.org> writes: > On 10/18/2016 10:07 AM, Punit Agrawal wrote: >> Although ghes_proc tests for error while reading the error status, it >> always return success (0). Fix this by propagating the return value. >> >> Fixes: d334a49113a4a33 ("ACPI, APEI, Generic Hardware Error Source memory error support") >> Signed-of-by: Punit Agrawal <punit.agrawa.@arm.com> >> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> >> Cc: Len Brown <lenb@kernel.org> > Tested-by: Tyler Baicar <tbaicar@codeaurora.org> Thanks for taking the patch for a spin. Hopefully Rafael can pick the patch as a fixes for the next rc. > > Thanks, > Tyler >> --- >> drivers/acpi/apei/ghes.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c >> index f0a029e..0d099a2 100644 >> --- a/drivers/acpi/apei/ghes.c >> +++ b/drivers/acpi/apei/ghes.c >> @@ -662,7 +662,7 @@ static int ghes_proc(struct ghes *ghes) >> ghes_do_proc(ghes, ghes->estatus); >> out: >> ghes_clear_estatus(ghes); >> - return 0; >> + return rc; >> } >> static void ghes_add_timer(struct ghes *ghes) -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday, October 18, 2016 05:07:19 PM Punit Agrawal wrote: > Although ghes_proc tests for error while reading the error status, it > always return success (0). Fix this by propagating the return value. > > Fixes: d334a49113a4a33 ("ACPI, APEI, Generic Hardware Error Source memory error support") > Signed-of-by: Punit Agrawal <punit.agrawa.@arm.com> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> > Cc: Len Brown <lenb@kernel.org> > --- > drivers/acpi/apei/ghes.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c > index f0a029e..0d099a2 100644 > --- a/drivers/acpi/apei/ghes.c > +++ b/drivers/acpi/apei/ghes.c > @@ -662,7 +662,7 @@ static int ghes_proc(struct ghes *ghes) > ghes_do_proc(ghes, ghes->estatus); > out: > ghes_clear_estatus(ghes); > - return 0; > + return rc; > } > > static void ghes_add_timer(struct ghes *ghes) > Boris, all fine here? Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Oct 21, 2016 at 11:28:29PM +0200, Rafael J. Wysocki wrote: > Boris, all fine here? Short answer: Yeah, looks ok to me. Longer answer: I mean, this way ghes_proc() *actually* propagates the return value of ghes_read_estatus() and we don't do any processing if it failed. Which doesn't really tell me a whole lot about the actual processing, i.e., what ghes_do_proc() did. But ghes_do_proc() doesn't return anything and ghes_proc()'s retval is used only in contexts where we're asking whether something got processed or not. And for that, that fix is adequate. So: Reviewed-by: Borislav Petkov <bp@suse.de>
On Saturday, October 22, 2016 06:37:35 PM Borislav Petkov wrote: > On Fri, Oct 21, 2016 at 11:28:29PM +0200, Rafael J. Wysocki wrote: > > Boris, all fine here? > > Short answer: Yeah, looks ok to me. > > Longer answer: I mean, this way ghes_proc() *actually* propagates the > return value of ghes_read_estatus() and we don't do any processing if it > failed. > > Which doesn't really tell me a whole lot about the actual processing, > i.e., what ghes_do_proc() did. > > But ghes_do_proc() doesn't return anything and ghes_proc()'s retval is > used only in contexts where we're asking whether something got processed > or not. > > And for that, that fix is adequate. So: > > Reviewed-by: Borislav Petkov <bp@suse.de> Thanks! -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index f0a029e..0d099a2 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -662,7 +662,7 @@ static int ghes_proc(struct ghes *ghes) ghes_do_proc(ghes, ghes->estatus); out: ghes_clear_estatus(ghes); - return 0; + return rc; } static void ghes_add_timer(struct ghes *ghes)
Although ghes_proc tests for error while reading the error status, it always return success (0). Fix this by propagating the return value. Fixes: d334a49113a4a33 ("ACPI, APEI, Generic Hardware Error Source memory error support") Signed-of-by: Punit Agrawal <punit.agrawa.@arm.com> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net> Cc: Len Brown <lenb@kernel.org> --- drivers/acpi/apei/ghes.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)