Patchwork powerpc/pseries failed reconfig notifier chain call cleanup

login
register
mail settings
Submitter Nathan Fontenot
Date March 5, 2009, 7:53 p.m.
Message ID <49B02DB5.7020301@austin.ibm.com>
Download mbox | patch
Permalink /patch/24101/
State Accepted, archived
Commit c5785f9e1c1c07c791fdc471f5c7fda4a5855b0c
Delegated to: Benjamin Herrenschmidt
Headers show

Comments

Nathan Fontenot - March 5, 2009, 7:53 p.m.
The return code from invoking the notifier chain when updating the
ibm,dynamic-memory property is not handled properly. In failure
cases (rc == NOTIFY_BAD) we should be restoring the original value
of the property.  In success (rc == NOTIFY_OK) we should be returning
zero from the calling routine.
 
Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
---
 arch/powerpc/platforms/pseries/reconfig.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
Benjamin Herrenschmidt - March 11, 2009, 5:47 a.m.
On Thu, 2009-03-05 at 13:53 -0600, Nathan Fontenot wrote:
> The return code from invoking the notifier chain when updating the
> ibm,dynamic-memory property is not handled properly. In failure
> cases (rc == NOTIFY_BAD) we should be restoring the original value
> of the property.  In success (rc == NOTIFY_OK) we should be returning
> zero from the calling routine.

This is actually not clear to me ... if the memory has been added or
removed, we must make sure the device-tree is up to date... ie, we can't
tell the firmware that we failed can we ?

Ben.

> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
> ---
>  arch/powerpc/platforms/pseries/reconfig.c |    6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6/arch/powerpc/platforms/pseries/reconfig.c
> ===================================================================
> --- linux-2.6.orig/arch/powerpc/platforms/pseries/reconfig.c	2008-10-23 22:29:24.000000000 -0500
> +++ linux-2.6/arch/powerpc/platforms/pseries/reconfig.c	2009-03-05 13:20:00.000000000 -0600
> @@ -468,9 +468,13 @@
>  
>  		rc = blocking_notifier_call_chain(&pSeries_reconfig_chain,
>  						  action, value);
> +		if (rc == NOTIFY_BAD) {
> +			rc = prom_update_property(np, oldprop, newprop);
> +			return -ENOMEM;
> +		}
>  	}
>  
> -	return rc;
> +	return 0;
>  }
>  
>  /**
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
Nathan Fontenot - March 11, 2009, 3:14 p.m.
Benjamin Herrenschmidt wrote:
> On Thu, 2009-03-05 at 13:53 -0600, Nathan Fontenot wrote:
>> The return code from invoking the notifier chain when updating the
>> ibm,dynamic-memory property is not handled properly. In failure
>> cases (rc == NOTIFY_BAD) we should be restoring the original value
>> of the property.  In success (rc == NOTIFY_OK) we should be returning
>> zero from the calling routine.
> 
> This is actually not clear to me ... if the memory has been added or
> removed, we must make sure the device-tree is up to date... ie, we can't
> tell the firmware that we failed can we ?
> 

Once the memory is added or removed the device tree is updated to reflect
the change.  The case for systems where the memory in the device tree is
specified in the ibm,dynamic-reconfiguration-memory/ibm,dynamic-memory
property is slightly different.  Because it is a property that is being
updated (as opposed to addition or removal of a device tree node for
memory specified as memory@XXXX nodes) The kernel updates the property
in the device tree then invokes the notifier chain.  If anyone on the
notifier chain returns a failure we should restore the property to its
previous value.  I think that part is understood.

The main user (and probably only user) of this interface is the drmgr
tool that handles DLPAR of memory and other conmponents.  The drmgr
tool tries to update the property after acquiring it from firmware. If
the property update fails, drmgr cleans up and returns the memory to
firmware.  This update ensures that the device tree property is not left
in a state that implies that the system owns the memory.

Hope that helps.

-Nathan

> Ben.
> 
>> Signed-off-by: Nathan Fontenot <nfont@austin.ibm.com>
>> ---
>>  arch/powerpc/platforms/pseries/reconfig.c |    6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> Index: linux-2.6/arch/powerpc/platforms/pseries/reconfig.c
>> ===================================================================
>> --- linux-2.6.orig/arch/powerpc/platforms/pseries/reconfig.c	2008-10-23 22:29:24.000000000 -0500
>> +++ linux-2.6/arch/powerpc/platforms/pseries/reconfig.c	2009-03-05 13:20:00.000000000 -0600
>> @@ -468,9 +468,13 @@
>>  
>>  		rc = blocking_notifier_call_chain(&pSeries_reconfig_chain,
>>  						  action, value);
>> +		if (rc == NOTIFY_BAD) {
>> +			rc = prom_update_property(np, oldprop, newprop);
>> +			return -ENOMEM;
>> +		}
>>  	}
>>  
>> -	return rc;
>> +	return 0;
>>  }
>>  
>>  /**
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@ozlabs.org
>> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>

Patch

Index: linux-2.6/arch/powerpc/platforms/pseries/reconfig.c
===================================================================
--- linux-2.6.orig/arch/powerpc/platforms/pseries/reconfig.c	2008-10-23 22:29:24.000000000 -0500
+++ linux-2.6/arch/powerpc/platforms/pseries/reconfig.c	2009-03-05 13:20:00.000000000 -0600
@@ -468,9 +468,13 @@ 
 
 		rc = blocking_notifier_call_chain(&pSeries_reconfig_chain,
 						  action, value);
+		if (rc == NOTIFY_BAD) {
+			rc = prom_update_property(np, oldprop, newprop);
+			return -ENOMEM;
+		}
 	}
 
-	return rc;
+	return 0;
 }
 
 /**