diff mbox series

[1/3] powerpc/vphn: Check for error from hcall_vphn

Message ID 20190822144235.19398-2-srikar@linux.vnet.ibm.com (mailing list archive)
State Superseded
Headers show
Series Early node associativity | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch next (0e4523c0b4f64eaf7abe59e143e6bdf8f972acff)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 4 warnings, 0 checks, 37 lines checked

Commit Message

Srikar Dronamraju Aug. 22, 2019, 2:42 p.m. UTC
There is no point in unpacking associativity, if
H_HOME_NODE_ASSOCIATIVITY hcall has returned an error.

Also added error messages for H_PARAMETER and default case in
vphn_get_associativity.

Signed-off-by: Srikar Dronamraju <srikar@linux.vnet.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>
Cc: Nicholas Piggin <npiggin@gmail.com>
Cc: Nathan Lynch <nathanl@linux.ibm.com>
Cc: linuxppc-dev@lists.ozlabs.org
Reported-by: Satheesh Rajendran <sathnaga@linux.vnet.ibm.com>
Reported-by: Abdul Haleem <abdhalee@linux.vnet.ibm.com>
---
 arch/powerpc/mm/numa.c                | 16 +++++++++++++---
 arch/powerpc/platforms/pseries/vphn.c |  3 ++-
 2 files changed, 15 insertions(+), 4 deletions(-)

Comments

Nathan Lynch Aug. 22, 2019, 4:41 p.m. UTC | #1
Hi Srikar,

Srikar Dronamraju <srikar@linux.vnet.ibm.com> writes:
> There is no point in unpacking associativity, if
> H_HOME_NODE_ASSOCIATIVITY hcall has returned an error.
>
> Also added error messages for H_PARAMETER and default case in
> vphn_get_associativity.

These are two logical changes and should be separated IMO.


> diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
> index 50d68d2..88b5157 100644
> --- a/arch/powerpc/mm/numa.c
> +++ b/arch/powerpc/mm/numa.c
> @@ -1191,6 +1191,10 @@ static long vphn_get_associativity(unsigned long cpu,
>  				VPHN_FLAG_VCPU, associativity);
>  
>  	switch (rc) {
> +	case H_SUCCESS:
> +		dbg("VPHN hcall succeeded. Reset polling...\n");
> +		timed_topology_update(0);
> +		break;
>  	case H_FUNCTION:
>  		printk_once(KERN_INFO
>  			"VPHN is not supported. Disabling polling...\n");
> @@ -1202,9 +1206,15 @@ static long vphn_get_associativity(unsigned long cpu,
>  			"preventing VPHN. Disabling polling...\n");
>  		stop_topology_update();
>  		break;
> -	case H_SUCCESS:
> -		dbg("VPHN hcall succeeded. Reset polling...\n");
> -		timed_topology_update(0);
> +	case H_PARAMETER:
> +		printk(KERN_ERR
> +			"hcall_vphn() was passed an invalid parameter."
> +			"Disabling polling...\n");

This will come out as:

hcall_vphn() was passed an invalid parameter.Disabling polling...
                                             ^

And it's misleading to say VPHN polling is being disabled when this case
does not invoke stop_topology_update().

> +		break;
> +	default:
> +		printk(KERN_ERR
> +			"hcall_vphn() returned %ld. Disabling polling \n", rc);
> +		stop_topology_update();
>  		break;

Any added prints in this routine must be _once or _ratelimited to avoid
log floods. Also use the pr_ APIs instead of printk please.
diff mbox series

Patch

diff --git a/arch/powerpc/mm/numa.c b/arch/powerpc/mm/numa.c
index 50d68d2..88b5157 100644
--- a/arch/powerpc/mm/numa.c
+++ b/arch/powerpc/mm/numa.c
@@ -1191,6 +1191,10 @@  static long vphn_get_associativity(unsigned long cpu,
 				VPHN_FLAG_VCPU, associativity);
 
 	switch (rc) {
+	case H_SUCCESS:
+		dbg("VPHN hcall succeeded. Reset polling...\n");
+		timed_topology_update(0);
+		break;
 	case H_FUNCTION:
 		printk_once(KERN_INFO
 			"VPHN is not supported. Disabling polling...\n");
@@ -1202,9 +1206,15 @@  static long vphn_get_associativity(unsigned long cpu,
 			"preventing VPHN. Disabling polling...\n");
 		stop_topology_update();
 		break;
-	case H_SUCCESS:
-		dbg("VPHN hcall succeeded. Reset polling...\n");
-		timed_topology_update(0);
+	case H_PARAMETER:
+		printk(KERN_ERR
+			"hcall_vphn() was passed an invalid parameter."
+			"Disabling polling...\n");
+		break;
+	default:
+		printk(KERN_ERR
+			"hcall_vphn() returned %ld. Disabling polling \n", rc);
+		stop_topology_update();
 		break;
 	}
 
diff --git a/arch/powerpc/platforms/pseries/vphn.c b/arch/powerpc/platforms/pseries/vphn.c
index 3f07bf6..cca474a 100644
--- a/arch/powerpc/platforms/pseries/vphn.c
+++ b/arch/powerpc/platforms/pseries/vphn.c
@@ -82,7 +82,8 @@  long hcall_vphn(unsigned long cpu, u64 flags, __be32 *associativity)
 	long retbuf[PLPAR_HCALL9_BUFSIZE] = {0};
 
 	rc = plpar_hcall9(H_HOME_NODE_ASSOCIATIVITY, retbuf, flags, cpu);
-	vphn_unpack_associativity(retbuf, associativity);
+	if (rc == H_SUCCESS)
+		vphn_unpack_associativity(retbuf, associativity);
 
 	return rc;
 }