Patchwork powerpc/eeh: Fix undefined variable

login
register
mail settings
Submitter Mike Qiu
Date Aug. 7, 2013, 2:24 a.m.
Message ID <1375842286-26521-1-git-send-email-qiudayu@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/265305/
State Superseded
Headers show

Comments

Mike Qiu - Aug. 7, 2013, 2:24 a.m.
'pe_no' hasn't been defined, it should be an typo error,
it should be 'frozen_pe_no'.

Also '__func__' should be added to IODA_EEH_DBG(),

Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
---
 arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Gavin Shan - Aug. 7, 2013, 2:34 a.m.
On Tue, Aug 06, 2013 at 10:24:46PM -0400, Mike Qiu wrote:
>'pe_no' hasn't been defined, it should be an typo error,
>it should be 'frozen_pe_no'.
>
>Also '__func__' should be added to IODA_EEH_DBG(),
>
>Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>

Acked-by: Gavin Shan <shangw@linux.vnet.ibm.com>

>---
> arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>index 0cd1c4a..a49bee7 100644
>--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>@@ -843,7 +843,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
> 		 * specific PHB.
> 		 */
> 		IODA_EEH_DBG("%s: Error (%d, %d, %d) on PHB#%x\n",
>-			err_type, severity, pe_no, hose->global_number);
>+			__func__, err_type, severity,
>+			frozen_pe_no, hose->global_number);
> 		switch (err_type) {
> 		case OPAL_EEH_IOC_ERROR:
> 			if (severity == OPAL_EEH_SEV_IOC_DEAD) {

Thanks,
Gavin
Michael Ellerman - Aug. 7, 2013, 5:11 a.m.
On Tue, Aug 06, 2013 at 10:24:46PM -0400, Mike Qiu wrote:
> 'pe_no' hasn't been defined, it should be an typo error,
> it should be 'frozen_pe_no'.
> 
> Also '__func__' should be added to IODA_EEH_DBG(),
> 
> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
> ---
>  arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
> index 0cd1c4a..a49bee7 100644
> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
> @@ -843,7 +843,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>  		 * specific PHB.
>  		 */
>  		IODA_EEH_DBG("%s: Error (%d, %d, %d) on PHB#%x\n",
> -			err_type, severity, pe_no, hose->global_number);
> +			__func__, err_type, severity,
> +			frozen_pe_no, hose->global_number);

Why is it using a custom macro? If you use pr_devel() or similar you
avoid these bugs, because the argument list is always expanded.

cheers
Gavin Shan - Aug. 7, 2013, 5:25 a.m.
On Wed, Aug 07, 2013 at 03:11:24PM +1000, Michael Ellerman wrote:
>On Tue, Aug 06, 2013 at 10:24:46PM -0400, Mike Qiu wrote:
>> 'pe_no' hasn't been defined, it should be an typo error,
>> it should be 'frozen_pe_no'.
>> 
>> Also '__func__' should be added to IODA_EEH_DBG(),
>> 
>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>> ---
>>  arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> index 0cd1c4a..a49bee7 100644
>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>> @@ -843,7 +843,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>>  		 * specific PHB.
>>  		 */
>>  		IODA_EEH_DBG("%s: Error (%d, %d, %d) on PHB#%x\n",
>> -			err_type, severity, pe_no, hose->global_number);
>> +			__func__, err_type, severity,
>> +			frozen_pe_no, hose->global_number);
>
>Why is it using a custom macro? If you use pr_devel() or similar you
>avoid these bugs, because the argument list is always expanded.
>

The custom macro at least can save some CPU cycles, but that's not
safe as you mentioned. It's resonable to use pr_devel() here.

Mike, could you help to replace IODA_EEH_DBG() with pr_devel() as
Michael suggested?

Thanks,
Gavin
Mike Qiu - Aug. 7, 2013, 5:37 a.m.
于 2013/8/7 13:25, Gavin Shan 写道:
> On Wed, Aug 07, 2013 at 03:11:24PM +1000, Michael Ellerman wrote:
>> On Tue, Aug 06, 2013 at 10:24:46PM -0400, Mike Qiu wrote:
>>> 'pe_no' hasn't been defined, it should be an typo error,
>>> it should be 'frozen_pe_no'.
>>>
>>> Also '__func__' should be added to IODA_EEH_DBG(),
>>>
>>> Signed-off-by: Mike Qiu <qiudayu@linux.vnet.ibm.com>
>>> ---
>>>   arch/powerpc/platforms/powernv/eeh-ioda.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>> index 0cd1c4a..a49bee7 100644
>>> --- a/arch/powerpc/platforms/powernv/eeh-ioda.c
>>> +++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
>>> @@ -843,7 +843,8 @@ static int ioda_eeh_next_error(struct eeh_pe **pe)
>>>   		 * specific PHB.
>>>   		 */
>>>   		IODA_EEH_DBG("%s: Error (%d, %d, %d) on PHB#%x\n",
>>> -			err_type, severity, pe_no, hose->global_number);
>>> +			__func__, err_type, severity,
>>> +			frozen_pe_no, hose->global_number);
>> Why is it using a custom macro? If you use pr_devel() or similar you
>> avoid these bugs, because the argument list is always expanded.
>>
> The custom macro at least can save some CPU cycles, but that's not
> safe as you mentioned. It's resonable to use pr_devel() here.
>
> Mike, could you help to replace IODA_EEH_DBG() with pr_devel() as
> Michael suggested?
OK, I will change the patch in V2

Thanks
Mike
>
> Thanks,
> Gavin
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
>
>

Patch

diff --git a/arch/powerpc/platforms/powernv/eeh-ioda.c b/arch/powerpc/platforms/powernv/eeh-ioda.c
index 0cd1c4a..a49bee7 100644
--- a/arch/powerpc/platforms/powernv/eeh-ioda.c
+++ b/arch/powerpc/platforms/powernv/eeh-ioda.c
@@ -843,7 +843,8 @@  static int ioda_eeh_next_error(struct eeh_pe **pe)
 		 * specific PHB.
 		 */
 		IODA_EEH_DBG("%s: Error (%d, %d, %d) on PHB#%x\n",
-			err_type, severity, pe_no, hose->global_number);
+			__func__, err_type, severity,
+			frozen_pe_no, hose->global_number);
 		switch (err_type) {
 		case OPAL_EEH_IOC_ERROR:
 			if (severity == OPAL_EEH_SEV_IOC_DEAD) {