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 <[email protected]>
Cc: "Rafael J. Wysocki" <[email protected]>
Cc: Len Brown <[email protected]>
---
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)
--
2.9.3
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 <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Len Brown <[email protected]>
Tested-by: Tyler Baicar <[email protected]>
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)
--
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.
"Baicar, Tyler" <[email protected]> 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 <[email protected]>
>> Cc: "Rafael J. Wysocki" <[email protected]>
>> Cc: Len Brown <[email protected]>
> Tested-by: Tyler Baicar <[email protected]>
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)
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 <[email protected]>
> Cc: "Rafael J. Wysocki" <[email protected]>
> Cc: Len Brown <[email protected]>
> ---
> 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
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 <[email protected]>
--
Regards/Gruss,
Boris.
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
--
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 <[email protected]>
Thanks!