| Message ID | 20191127072035.4283-1-anju@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [v3] platforms/powernv: Avoid re-registration of imc debugfs directory | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (2ec2260ce7bce5eb6a8ced0bb78d75c1b3eca306) |
| snowpatch_ozlabs/build-ppc64le | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64be | success | Build succeeded |
| snowpatch_ozlabs/build-ppc64e | success | Build succeeded |
| snowpatch_ozlabs/build-pmac32 | success | Build succeeded |
| snowpatch_ozlabs/checkpatch | warning | total: 0 errors, 1 warnings, 0 checks, 112 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
"Anju T Sudhakar" <anju@linux.vnet.ibm.com> wrote on 27/11/2019 12:50:35 PM: > From: "Anju T Sudhakar" <anju@linux.vnet.ibm.com> > To: mpe@ellerman.id.au > Cc: linuxppc-dev@lists.ozlabs.org, anju@linux.vnet.ibm.com, > Nageswara R Sastry/India/IBM@IBMIN > Date: 27/11/2019 12:50 PM > Subject: [PATCH v3] platforms/powernv: Avoid re-registration of imc > debugfs directory > > 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> > --- > Changes from v2 -> v3: > > * Invoke export_imc_mode_and_cmd(), which does the imc debugfs > directory registration and deletion, from the probe fucntion. > * Change the return type of imc_pmu_create() to get the > control block address for nest units in the probe > function > * Remove unnecessary comments Tested-by: Nageswara R Sastry <nasastry@in.ibm.com> Not seeing any Kernel OOPs/warnings in dmesg # cat /sys/kernel/debug/powerpc/imc/* 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 > --- > 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 e04b206..3b4518f 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; > @@ -293,9 +286,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) > @@ -303,10 +300,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(); > -- > 1.8.3.1 >
On Wed, 2019-11-27 at 07:20:35 UTC, Anju T Sudhakar wrote: > 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> Applied to powerpc fixes, thanks. https://git.kernel.org/powerpc/c/48e626ac85b43cc589dd1b3b8004f7f85f03544d cheers
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c index e04b206..3b4518f 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; @@ -293,9 +286,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) @@ -303,10 +300,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();
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> --- Changes from v2 -> v3: * Invoke export_imc_mode_and_cmd(), which does the imc debugfs directory registration and deletion, from the probe fucntion. * Change the return type of imc_pmu_create() to get the control block address for nest units in the probe function * Remove unnecessary comments --- arch/powerpc/platforms/powernv/opal-imc.c | 39 +++++++++++++------------------ 1 file changed, 16 insertions(+), 23 deletions(-)