[PATCH v3 -next] powerpc/pseries/memory-hotplug: Fix return value type of find_aa_index

[PATCH v3 -next] powerpc/pseries/memory-hotplug: Fix return value type of find_aa_index

[YueHaibing]

'aa_index' is defined as an unsigned value, but find_aa_index
may return -1 when dlpar_clone_property fails. So change 
find_aa_index return value type to bool, which indicate 'aa_index'
whether found or not.

Fixes: c05a5a40969e ("powerpc/pseries: Dynamic add entires to associativity lookup array")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
v3: change find_aa_index return type to bool
v2: use 'rc' track the validation of aa_index
---
 arch/powerpc/platforms/pseries/hotplug-memory.c | 61 ++++++++++++-------------
 1 file changed, 28 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
index d26a771..4db510f 100644
--- a/arch/powerpc/platforms/pseries/hotplug-memory.c
+++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
@@ -101,11 +101,12 @@ static struct property *dlpar_clone_property(struct property *prop,
 	return new_prop;
 }
 
-static u32 find_aa_index(struct device_node *dr_node,
-			 struct property *ala_prop, const u32 *lmb_assoc)
+static bool find_aa_index(struct device_node *dr_node,
+			 struct property *ala_prop,
+			 const u32 *lmb_assoc, u32 *aa_index)
 {
-	u32 *assoc_arrays;
-	u32 aa_index;
+	u32 *assoc_arrays, new_prop_size;
+	struct property *new_prop;
 	int aa_arrays, aa_array_entries, aa_array_sz;
 	int i, index;
 
@@ -121,46 +122,39 @@ static u32 find_aa_index(struct device_node *dr_node,
 	aa_array_entries = be32_to_cpu(assoc_arrays[1]);
 	aa_array_sz = aa_array_entries * sizeof(u32);
 
-	aa_index = -1;
 	for (i = 0; i < aa_arrays; i++) {
 		index = (i * aa_array_entries) + 2;
 
 		if (memcmp(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz))
 			continue;
 
-		aa_index = i;
-		break;
+		*aa_index = i;
+		return true;
 	}
 
-	if (aa_index == -1) {
-		struct property *new_prop;
-		u32 new_prop_size;
-
-		new_prop_size = ala_prop->length + aa_array_sz;
-		new_prop = dlpar_clone_property(ala_prop, new_prop_size);
-		if (!new_prop)
-			return -1;
-
-		assoc_arrays = new_prop->value;
+	new_prop_size = ala_prop->length + aa_array_sz;
+	new_prop = dlpar_clone_property(ala_prop, new_prop_size);
+	if (!new_prop)
+		return false;
 
-		/* increment the number of entries in the lookup array */
-		assoc_arrays[0] = cpu_to_be32(aa_arrays + 1);
+	assoc_arrays = new_prop->value;
 
-		/* copy the new associativity into the lookup array */
-		index = aa_arrays * aa_array_entries + 2;
-		memcpy(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz);
+	/* increment the number of entries in the lookup array */
+	assoc_arrays[0] = cpu_to_be32(aa_arrays + 1);
 
-		of_update_property(dr_node, new_prop);
+	/* copy the new associativity into the lookup array */
+	index = aa_arrays * aa_array_entries + 2;
+	memcpy(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz);
 
-		/*
-		 * The associativity lookup array index for this lmb is
-		 * number of entries - 1 since we added its associativity
-		 * to the end of the lookup array.
-		 */
-		aa_index = be32_to_cpu(assoc_arrays[0]) - 1;
-	}
+	of_update_property(dr_node, new_prop);
 
-	return aa_index;
+	/*
+	 * The associativity lookup array index for this lmb is
+	 * number of entries - 1 since we added its associativity
+	 * to the end of the lookup array.
+	 */
+	*aa_index = be32_to_cpu(assoc_arrays[0]) - 1;
+	return true;
 }
 
 static int update_lmb_associativity_index(struct drmem_lmb *lmb)
@@ -169,6 +163,7 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
 	struct property *ala_prop;
 	const u32 *lmb_assoc;
 	u32 aa_index;
+	bool is_found;
 
 	parent = of_find_node_by_path("/");
 	if (!parent)
@@ -200,11 +195,11 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
 		return -ENODEV;
 	}
 
-	aa_index = find_aa_index(dr_node, ala_prop, lmb_assoc);
+	is_found = find_aa_index(dr_node, ala_prop, lmb_assoc, &aa_index);
 
 	dlpar_free_cc_nodes(lmb_node);
 
-	if (aa_index < 0) {
+	if (!is_found) {
 		pr_err("Could not find LMB associativity\n");
 		return -1;
 	}
-- 
2.7.0

Re: [PATCH v3 -next] powerpc/pseries/memory-hotplug: Fix return value type of find_aa_index

[Nathan Fontenot]

On 10/09/2018 08:59 AM, YueHaibing wrote:
> 'aa_index' is defined as an unsigned value, but find_aa_index
> may return -1 when dlpar_clone_property fails. So change 
> find_aa_index return value type to bool, which indicate 'aa_index'
> whether found or not.
> 
> Fixes: c05a5a40969e ("powerpc/pseries: Dynamic add entires to associativity lookup array")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>

Reviewed-by: Nathan Fontenot nfont@linux.vnet.ibm.com>
 
> ---
> v3: change find_aa_index return type to bool
> v2: use 'rc' track the validation of aa_index
> ---
>  arch/powerpc/platforms/pseries/hotplug-memory.c | 61 ++++++++++++-------------
>  1 file changed, 28 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/hotplug-memory.c b/arch/powerpc/platforms/pseries/hotplug-memory.c
> index d26a771..4db510f 100644
> --- a/arch/powerpc/platforms/pseries/hotplug-memory.c
> +++ b/arch/powerpc/platforms/pseries/hotplug-memory.c
> @@ -101,11 +101,12 @@ static struct property *dlpar_clone_property(struct property *prop,
>  	return new_prop;
>  }
> 
> -static u32 find_aa_index(struct device_node *dr_node,
> -			 struct property *ala_prop, const u32 *lmb_assoc)
> +static bool find_aa_index(struct device_node *dr_node,
> +			 struct property *ala_prop,
> +			 const u32 *lmb_assoc, u32 *aa_index)
>  {
> -	u32 *assoc_arrays;
> -	u32 aa_index;
> +	u32 *assoc_arrays, new_prop_size;
> +	struct property *new_prop;
>  	int aa_arrays, aa_array_entries, aa_array_sz;
>  	int i, index;
> 
> @@ -121,46 +122,39 @@ static u32 find_aa_index(struct device_node *dr_node,
>  	aa_array_entries = be32_to_cpu(assoc_arrays[1]);
>  	aa_array_sz = aa_array_entries * sizeof(u32);
> 
> -	aa_index = -1;
>  	for (i = 0; i < aa_arrays; i++) {
>  		index = (i * aa_array_entries) + 2;
> 
>  		if (memcmp(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz))
>  			continue;
> 
> -		aa_index = i;
> -		break;
> +		*aa_index = i;
> +		return true;
>  	}
> 
> -	if (aa_index == -1) {
> -		struct property *new_prop;
> -		u32 new_prop_size;
> -
> -		new_prop_size = ala_prop->length + aa_array_sz;
> -		new_prop = dlpar_clone_property(ala_prop, new_prop_size);
> -		if (!new_prop)
> -			return -1;
> -
> -		assoc_arrays = new_prop->value;
> +	new_prop_size = ala_prop->length + aa_array_sz;
> +	new_prop = dlpar_clone_property(ala_prop, new_prop_size);
> +	if (!new_prop)
> +		return false;
> 
> -		/* increment the number of entries in the lookup array */
> -		assoc_arrays[0] = cpu_to_be32(aa_arrays + 1);
> +	assoc_arrays = new_prop->value;
> 
> -		/* copy the new associativity into the lookup array */
> -		index = aa_arrays * aa_array_entries + 2;
> -		memcpy(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz);
> +	/* increment the number of entries in the lookup array */
> +	assoc_arrays[0] = cpu_to_be32(aa_arrays + 1);
> 
> -		of_update_property(dr_node, new_prop);
> +	/* copy the new associativity into the lookup array */
> +	index = aa_arrays * aa_array_entries + 2;
> +	memcpy(&assoc_arrays[index], &lmb_assoc[1], aa_array_sz);
> 
> -		/*
> -		 * The associativity lookup array index for this lmb is
> -		 * number of entries - 1 since we added its associativity
> -		 * to the end of the lookup array.
> -		 */
> -		aa_index = be32_to_cpu(assoc_arrays[0]) - 1;
> -	}
> +	of_update_property(dr_node, new_prop);
> 
> -	return aa_index;
> +	/*
> +	 * The associativity lookup array index for this lmb is
> +	 * number of entries - 1 since we added its associativity
> +	 * to the end of the lookup array.
> +	 */
> +	*aa_index = be32_to_cpu(assoc_arrays[0]) - 1;
> +	return true;
>  }
> 
>  static int update_lmb_associativity_index(struct drmem_lmb *lmb)
> @@ -169,6 +163,7 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
>  	struct property *ala_prop;
>  	const u32 *lmb_assoc;
>  	u32 aa_index;
> +	bool is_found;
> 
>  	parent = of_find_node_by_path("/");
>  	if (!parent)
> @@ -200,11 +195,11 @@ static int update_lmb_associativity_index(struct drmem_lmb *lmb)
>  		return -ENODEV;
>  	}
> 
> -	aa_index = find_aa_index(dr_node, ala_prop, lmb_assoc);
> +	is_found = find_aa_index(dr_node, ala_prop, lmb_assoc, &aa_index);
> 
>  	dlpar_free_cc_nodes(lmb_node);
> 
> -	if (aa_index < 0) {
> +	if (!is_found) {
>  		pr_err("Could not find LMB associativity\n");
>  		return -1;
>  	}
> 

Re: [v3, -next] powerpc/pseries/memory-hotplug: Fix return value type of find_aa_index

[Michael Ellerman]

On Tue, 2018-10-09 at 13:59:13 UTC, YueHaibing wrote:
> 'aa_index' is defined as an unsigned value, but find_aa_index
> may return -1 when dlpar_clone_property fails. So change 
> find_aa_index return value type to bool, which indicate 'aa_index'
> whether found or not.
> 
> Fixes: c05a5a40969e ("powerpc/pseries: Dynamic add entires to associativity lookup array")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> Reviewed-by: Nathan Fontenot nfont@linux.vnet.ibm.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/b45e9d761ba2d60044b610297e3ef9

cheers