[F,1/1] powerpc/powernv: Avoid re-registration of imc debugfs directory
diff mbox series

Message ID 1580385786-21047-2-git-send-email-frank.heimes@canonical.com
State New
Headers show
Series
  • [F,1/1] powerpc/powernv: Avoid re-registration of imc debugfs directory
Related show

Commit Message

Frank Heimes Jan. 30, 2020, 12:03 p.m. UTC
From: Anju T Sudhakar <anju@linux.vnet.ibm.com>

BugLink: https://bugs.launchpad.net/bugs/1861379

export_imc_mode_and_cmd() function which creates the debugfs interface
for imc-mode and imc-command, is invoked when each nest pmu units is
registered.

When the first nest pmu unit is registered, export_imc_mode_and_cmd()
creates 'imc' directory under `/debug/powerpc/`. In the subsequent
invocations debugfs_create_dir() function returns, since the directory
already exists.

The recent commit <c33d442328f55> (debugfs: make error message a bit
more verbose), throws a warning if we try to invoke
`debugfs_create_dir()` with an already existing directory name.

Address this warning by making the debugfs directory registration in
the opal_imc_counters_probe() function, i.e invoke
export_imc_mode_and_cmd() function from the probe function.

Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Tested-by: Nageswara R Sastry <nasastry@in.ibm.com>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20191127072035.4283-1-anju@linux.vnet.ibm.com
(cherry picked from commit 48e626ac85b43cc589dd1b3b8004f7f85f03544d)
Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
---
 arch/powerpc/platforms/powernv/opal-imc.c | 39 +++++++++++++------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

Comments

Colin Ian King Jan. 30, 2020, 12:09 p.m. UTC | #1
On 30/01/2020 12:03, frank.heimes@canonical.com wrote:
> From: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1861379
> 
> export_imc_mode_and_cmd() function which creates the debugfs interface
> for imc-mode and imc-command, is invoked when each nest pmu units is
> registered.
> 
> When the first nest pmu unit is registered, export_imc_mode_and_cmd()
> creates 'imc' directory under `/debug/powerpc/`. In the subsequent
> invocations debugfs_create_dir() function returns, since the directory
> already exists.
> 
> The recent commit <c33d442328f55> (debugfs: make error message a bit
> more verbose), throws a warning if we try to invoke
> `debugfs_create_dir()` with an already existing directory name.
> 
> Address this warning by making the debugfs directory registration in
> the opal_imc_counters_probe() function, i.e invoke
> export_imc_mode_and_cmd() function from the probe function.
> 
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Tested-by: Nageswara R Sastry <nasastry@in.ibm.com>
> Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
> Link: https://lore.kernel.org/r/20191127072035.4283-1-anju@linux.vnet.ibm.com
> (cherry picked from commit 48e626ac85b43cc589dd1b3b8004f7f85f03544d)
> Signed-off-by: Frank Heimes <frank.heimes@canonical.com>
> ---
>  arch/powerpc/platforms/powernv/opal-imc.c | 39 +++++++++++++------------------
>  1 file changed, 16 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index 7ccc5c8..000b350 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -59,10 +59,6 @@ static void export_imc_mode_and_cmd(struct device_node *node,
>  
>  	imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
>  
> -	/*
> -	 * Return here, either because 'imc' directory already exists,
> -	 * Or failed to create a new one.
> -	 */
>  	if (!imc_debugfs_parent)
>  		return;
>  
> @@ -135,7 +131,6 @@ static int imc_get_mem_addr_nest(struct device_node *node,
>  	}
>  
>  	pmu_ptr->imc_counter_mmaped = true;
> -	export_imc_mode_and_cmd(node, pmu_ptr);
>  	kfree(base_addr_arr);
>  	kfree(chipid_arr);
>  	return 0;
> @@ -151,7 +146,7 @@ static int imc_get_mem_addr_nest(struct device_node *node,
>   *		    and domain as the inputs.
>   * Allocates memory for the struct imc_pmu, sets up its domain, size and offsets
>   */
> -static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
> +static struct imc_pmu *imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
>  {
>  	int ret = 0;
>  	struct imc_pmu *pmu_ptr;
> @@ -159,27 +154,23 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
>  
>  	/* Return for unknown domain */
>  	if (domain < 0)
> -		return -EINVAL;
> +		return NULL;
>  
>  	/* memory for pmu */
>  	pmu_ptr = kzalloc(sizeof(*pmu_ptr), GFP_KERNEL);
>  	if (!pmu_ptr)
> -		return -ENOMEM;
> +		return NULL;
>  
>  	/* Set the domain */
>  	pmu_ptr->domain = domain;
>  
>  	ret = of_property_read_u32(parent, "size", &pmu_ptr->counter_mem_size);
> -	if (ret) {
> -		ret = -EINVAL;
> +	if (ret)
>  		goto free_pmu;
> -	}
>  
>  	if (!of_property_read_u32(parent, "offset", &offset)) {
> -		if (imc_get_mem_addr_nest(parent, pmu_ptr, offset)) {
> -			ret = -EINVAL;
> +		if (imc_get_mem_addr_nest(parent, pmu_ptr, offset))
>  			goto free_pmu;
> -		}
>  	}
>  
>  	/* Function to register IMC pmu */
> @@ -190,14 +181,14 @@ static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
>  		if (pmu_ptr->domain == IMC_DOMAIN_NEST)
>  			kfree(pmu_ptr->mem_info);
>  		kfree(pmu_ptr);
> -		return ret;
> +		return NULL;
>  	}
>  
> -	return 0;
> +	return pmu_ptr;
>  
>  free_pmu:
>  	kfree(pmu_ptr);
> -	return ret;
> +	return NULL;
>  }
>  
>  static void disable_nest_pmu_counters(void)
> @@ -254,6 +245,7 @@ int get_max_nest_dev(void)
>  static int opal_imc_counters_probe(struct platform_device *pdev)
>  {
>  	struct device_node *imc_dev = pdev->dev.of_node;
> +	struct imc_pmu *pmu;
>  	int pmu_count = 0, domain;
>  	bool core_imc_reg = false, thread_imc_reg = false;
>  	u32 type;
> @@ -269,6 +261,7 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>  	}
>  
>  	for_each_compatible_node(imc_dev, NULL, IMC_DTB_UNIT_COMPAT) {
> +		pmu = NULL;
>  		if (of_property_read_u32(imc_dev, "type", &type)) {
>  			pr_warn("IMC Device without type property\n");
>  			continue;
> @@ -300,9 +293,13 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>  			break;
>  		}
>  
> -		if (!imc_pmu_create(imc_dev, pmu_count, domain)) {
> -			if (domain == IMC_DOMAIN_NEST)
> +		pmu = imc_pmu_create(imc_dev, pmu_count, domain);
> +		if (pmu != NULL) {
> +			if (domain == IMC_DOMAIN_NEST) {
> +				if (!imc_debugfs_parent)
> +					export_imc_mode_and_cmd(imc_dev, pmu);
>  				pmu_count++;
> +			}
>  			if (domain == IMC_DOMAIN_CORE)
>  				core_imc_reg = true;
>  			if (domain == IMC_DOMAIN_THREAD)
> @@ -310,10 +307,6 @@ static int opal_imc_counters_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	/* If none of the nest units are registered, remove debugfs interface */
> -	if (pmu_count == 0)
> -		debugfs_remove_recursive(imc_debugfs_parent);
> -
>  	/* If core imc is not registered, unregister thread-imc */
>  	if (!core_imc_reg && thread_imc_reg)
>  		unregister_thread_imc();
> 

Upstream cherry pick, addresses the issue, so..

Acked-by: Colin Ian King <colin.king@canonical.com>

Patch
diff mbox series

diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index 7ccc5c8..000b350 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -59,10 +59,6 @@  static void export_imc_mode_and_cmd(struct device_node *node,
 
 	imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
 
-	/*
-	 * Return here, either because 'imc' directory already exists,
-	 * Or failed to create a new one.
-	 */
 	if (!imc_debugfs_parent)
 		return;
 
@@ -135,7 +131,6 @@  static int imc_get_mem_addr_nest(struct device_node *node,
 	}
 
 	pmu_ptr->imc_counter_mmaped = true;
-	export_imc_mode_and_cmd(node, pmu_ptr);
 	kfree(base_addr_arr);
 	kfree(chipid_arr);
 	return 0;
@@ -151,7 +146,7 @@  static int imc_get_mem_addr_nest(struct device_node *node,
  *		    and domain as the inputs.
  * Allocates memory for the struct imc_pmu, sets up its domain, size and offsets
  */
-static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
+static struct imc_pmu *imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
 {
 	int ret = 0;
 	struct imc_pmu *pmu_ptr;
@@ -159,27 +154,23 @@  static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
 
 	/* Return for unknown domain */
 	if (domain < 0)
-		return -EINVAL;
+		return NULL;
 
 	/* memory for pmu */
 	pmu_ptr = kzalloc(sizeof(*pmu_ptr), GFP_KERNEL);
 	if (!pmu_ptr)
-		return -ENOMEM;
+		return NULL;
 
 	/* Set the domain */
 	pmu_ptr->domain = domain;
 
 	ret = of_property_read_u32(parent, "size", &pmu_ptr->counter_mem_size);
-	if (ret) {
-		ret = -EINVAL;
+	if (ret)
 		goto free_pmu;
-	}
 
 	if (!of_property_read_u32(parent, "offset", &offset)) {
-		if (imc_get_mem_addr_nest(parent, pmu_ptr, offset)) {
-			ret = -EINVAL;
+		if (imc_get_mem_addr_nest(parent, pmu_ptr, offset))
 			goto free_pmu;
-		}
 	}
 
 	/* Function to register IMC pmu */
@@ -190,14 +181,14 @@  static int imc_pmu_create(struct device_node *parent, int pmu_index, int domain)
 		if (pmu_ptr->domain == IMC_DOMAIN_NEST)
 			kfree(pmu_ptr->mem_info);
 		kfree(pmu_ptr);
-		return ret;
+		return NULL;
 	}
 
-	return 0;
+	return pmu_ptr;
 
 free_pmu:
 	kfree(pmu_ptr);
-	return ret;
+	return NULL;
 }
 
 static void disable_nest_pmu_counters(void)
@@ -254,6 +245,7 @@  int get_max_nest_dev(void)
 static int opal_imc_counters_probe(struct platform_device *pdev)
 {
 	struct device_node *imc_dev = pdev->dev.of_node;
+	struct imc_pmu *pmu;
 	int pmu_count = 0, domain;
 	bool core_imc_reg = false, thread_imc_reg = false;
 	u32 type;
@@ -269,6 +261,7 @@  static int opal_imc_counters_probe(struct platform_device *pdev)
 	}
 
 	for_each_compatible_node(imc_dev, NULL, IMC_DTB_UNIT_COMPAT) {
+		pmu = NULL;
 		if (of_property_read_u32(imc_dev, "type", &type)) {
 			pr_warn("IMC Device without type property\n");
 			continue;
@@ -300,9 +293,13 @@  static int opal_imc_counters_probe(struct platform_device *pdev)
 			break;
 		}
 
-		if (!imc_pmu_create(imc_dev, pmu_count, domain)) {
-			if (domain == IMC_DOMAIN_NEST)
+		pmu = imc_pmu_create(imc_dev, pmu_count, domain);
+		if (pmu != NULL) {
+			if (domain == IMC_DOMAIN_NEST) {
+				if (!imc_debugfs_parent)
+					export_imc_mode_and_cmd(imc_dev, pmu);
 				pmu_count++;
+			}
 			if (domain == IMC_DOMAIN_CORE)
 				core_imc_reg = true;
 			if (domain == IMC_DOMAIN_THREAD)
@@ -310,10 +307,6 @@  static int opal_imc_counters_probe(struct platform_device *pdev)
 		}
 	}
 
-	/* If none of the nest units are registered, remove debugfs interface */
-	if (pmu_count == 0)
-		debugfs_remove_recursive(imc_debugfs_parent);
-
 	/* If core imc is not registered, unregister thread-imc */
 	if (!core_imc_reg && thread_imc_reg)
 		unregister_thread_imc();