* [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory
@ 2019-08-21 4:09 Anju T Sudhakar
2019-08-21 4:46 ` Oliver O'Halloran
0 siblings, 1 reply; 4+ messages in thread
From: Anju T Sudhakar @ 2019-08-21 4:09 UTC (permalink / raw)
To: mpe; +Cc: anju, linuxppc-dev
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 lookup for an existing 'imc' directory,
and do not invoke debugfs_create_dir(), if the debugfs interface for
imc already exists.
This patch is based on:
https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/192979.html
Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Tested-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
---
arch/powerpc/platforms/powernv/opal-imc.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
index e04b20625cb9..fc2f0e60a44d 100644
--- a/arch/powerpc/platforms/powernv/opal-imc.c
+++ b/arch/powerpc/platforms/powernv/opal-imc.c
@@ -55,14 +55,19 @@ static void export_imc_mode_and_cmd(struct device_node *node,
static u64 loc, *imc_mode_addr, *imc_cmd_addr;
char mode[16], cmd[16];
u32 cb_offset;
+ struct dentry *dir = NULL;
struct imc_mem_info *ptr = pmu_ptr->mem_info;
+
+ /* Return, if 'imc' interface already exists */
+ dir = debugfs_lookup("imc", powerpc_debugfs_root);
+ if (dir) {
+ dput(dir);
+ return;
+ }
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.
- */
+ /* Return here, if failed to create the directory */
if (!imc_debugfs_parent)
return;
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory
2019-08-21 4:09 [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory Anju T Sudhakar
@ 2019-08-21 4:46 ` Oliver O'Halloran
2019-08-21 5:37 ` Anju T Sudhakar
0 siblings, 1 reply; 4+ messages in thread
From: Oliver O'Halloran @ 2019-08-21 4:46 UTC (permalink / raw)
To: Anju T Sudhakar; +Cc: linuxppc-dev
On Wed, Aug 21, 2019 at 2:10 PM Anju T Sudhakar <anju@linux.vnet.ibm.com> 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 lookup for an existing 'imc' directory,
> and do not invoke debugfs_create_dir(), if the debugfs interface for
> imc already exists.
>
> This patch is based on:
> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/192979.html
>
> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Tested-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/powernv/opal-imc.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
> index e04b20625cb9..fc2f0e60a44d 100644
> --- a/arch/powerpc/platforms/powernv/opal-imc.c
> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
> @@ -55,14 +55,19 @@ static void export_imc_mode_and_cmd(struct device_node *node,
> static u64 loc, *imc_mode_addr, *imc_cmd_addr;
> char mode[16], cmd[16];
> u32 cb_offset;
> + struct dentry *dir = NULL;
> struct imc_mem_info *ptr = pmu_ptr->mem_info;
>
> +
> + /* Return, if 'imc' interface already exists */
> + dir = debugfs_lookup("imc", powerpc_debugfs_root);
> + if (dir) {
> + dput(dir);
> + return;
> + }
> imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
Is there a reason why we create the debugfs directory here and not in
opal_imc_counters_probe()? There's logic to remove the debugfs
directory in _probe() already so it seems like a more natural place to
it.
>
> - /*
> - * Return here, either because 'imc' directory already exists,
> - * Or failed to create a new one.
> - */
> + /* Return here, if failed to create the directory */
> if (!imc_debugfs_parent)
> return;
>
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory
2019-08-21 4:46 ` Oliver O'Halloran
@ 2019-08-21 5:37 ` Anju T Sudhakar
2019-08-21 6:08 ` Oliver O'Halloran
0 siblings, 1 reply; 4+ messages in thread
From: Anju T Sudhakar @ 2019-08-21 5:37 UTC (permalink / raw)
To: Oliver O'Halloran; +Cc: linuxppc-dev
Hi,
On 8/21/19 10:16 AM, Oliver O'Halloran wrote:
> On Wed, Aug 21, 2019 at 2:10 PM Anju T Sudhakar <anju@linux.vnet.ibm.com> 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 lookup for an existing 'imc' directory,
>> and do not invoke debugfs_create_dir(), if the debugfs interface for
>> imc already exists.
>>
>> This patch is based on:
>> https://lists.ozlabs.org/pipermail/linuxppc-dev/2019-July/192979.html
>>
>> Signed-off-by: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>> Tested-by: Nageswara R Sastry <rnsastry@linux.vnet.ibm.com>
>> ---
>> arch/powerpc/platforms/powernv/opal-imc.c | 13 +++++++++----
>> 1 file changed, 9 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/powerpc/platforms/powernv/opal-imc.c b/arch/powerpc/platforms/powernv/opal-imc.c
>> index e04b20625cb9..fc2f0e60a44d 100644
>> --- a/arch/powerpc/platforms/powernv/opal-imc.c
>> +++ b/arch/powerpc/platforms/powernv/opal-imc.c
>> @@ -55,14 +55,19 @@ static void export_imc_mode_and_cmd(struct device_node *node,
>> static u64 loc, *imc_mode_addr, *imc_cmd_addr;
>> char mode[16], cmd[16];
>> u32 cb_offset;
>> + struct dentry *dir = NULL;
>> struct imc_mem_info *ptr = pmu_ptr->mem_info;
>>
>> +
>> + /* Return, if 'imc' interface already exists */
>> + dir = debugfs_lookup("imc", powerpc_debugfs_root);
>> + if (dir) {
>> + dput(dir);
>> + return;
>> + }
>> imc_debugfs_parent = debugfs_create_dir("imc", powerpc_debugfs_root);
> Is there a reason why we create the debugfs directory here and not in
> opal_imc_counters_probe()? There's logic to remove the debugfs
> directory in _probe() already so it seems like a more natural place to
> it.
>
Good point. But we can only create the parent directory,
i.e 'imc' directory in `_probe()` function and the entries can be
created only here. The reason is, this debugfs entries are only for
IMC nest units. So, to get the imc mode and command values from
the nest memory region we need the relevant offsets from the control
block structure.
Since imc_get_mem_addr_nest() function reads this address
for each chip, we invoke the function to create the debugfs
entries after this values are populated(i.e export_imc_mode_and_cmd() in
invoked by
imc_get_mem_addr_nest()).
Also, if we create the parent directory in `_probe()` function,
we need to track whether the entries(i.e imc_cmd and imc_mode) are
created or not.
Regards,
Anju
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory
2019-08-21 5:37 ` Anju T Sudhakar
@ 2019-08-21 6:08 ` Oliver O'Halloran
0 siblings, 0 replies; 4+ messages in thread
From: Oliver O'Halloran @ 2019-08-21 6:08 UTC (permalink / raw)
To: Anju T Sudhakar; +Cc: linuxppc-dev
On Wed, Aug 21, 2019 at 3:37 PM Anju T Sudhakar <anju@linux.vnet.ibm.com> wrote:
>
> Hi,
>
> On 8/21/19 10:16 AM, Oliver O'Halloran wrote:
> > Is there a reason why we create the debugfs directory here and not in
> > opal_imc_counters_probe()? There's logic to remove the debugfs
> > directory in _probe() already so it seems like a more natural place to
> > it.
> >
> Good point. But we can only create the parent directory,
> i.e 'imc' directory in `_probe()` function and the entries can be
> created only here.
I know the entries can't be added in the probe function and I'm not
suggesting they should be.
> The reason is, this debugfs entries are only for
> IMC nest units. So, to get the imc mode and command values from
> the nest memory region we need the relevant offsets from the control
> block structure.
>
> Since imc_get_mem_addr_nest() function reads this address
> for each chip, we invoke the function to create the debugfs
> entries after this values are populated(i.e export_imc_mode_and_cmd() in
> invoked by imc_get_mem_addr_nest()).
>
> Also, if we create the parent directory in `_probe()` function,
> we need to track whether the entries(i.e imc_cmd and imc_mode) are
> created or not.
I think you should be tracking this anyway so you can do proper
cleanup if a debugfs entry can't be added. The current approach of
nuking the entire debugfs directory is pretty questionable.
> Regards,
>
> Anju
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-08-21 6:10 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-21 4:09 [PATCH] platform/powernv: Avoid re-registration of imc debugfs directory Anju T Sudhakar
2019-08-21 4:46 ` Oliver O'Halloran
2019-08-21 5:37 ` Anju T Sudhakar
2019-08-21 6:08 ` Oliver O'Halloran
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).