Hi Srikar,

Srikar Dronamraju <sri...@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.

Reply via email to