Cleanup of -Wunused-const-variable in drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c

Cleanup of -Wunused-const-variable in drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c

[Nathan Huckleberry]

Hey all,

I'm looking into cleaning up ignored warnings in the kernel so we can
remove compiler flags to ignore warnings.

There's an unused variable 'mvpp2_dbgfs_prs_pmap_fops' in
mvpp2_debugfs.c. It looks like this code is for dumping useful
information into userspace. I'd like to either remove the variable or
dump it to userspace in the same way the other variables are.

Wanted to reach out for opinions on the best course of action before
submitting a patch.

https://github.com/ClangBuiltLinux/linux/issues/529

Thanks,
Nathan Huckleberry

Re: Cleanup of -Wunused-const-variable in drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c

[Maxime Chevallier]

Hello Nathan,

On Thu, 13 Jun 2019 10:53:05 -0700
Nathan Huckleberry <nhuck@google.com> wrote:

>Hey all,
>
>I'm looking into cleaning up ignored warnings in the kernel so we can
>remove compiler flags to ignore warnings.
>
>There's an unused variable 'mvpp2_dbgfs_prs_pmap_fops' in
>mvpp2_debugfs.c. It looks like this code is for dumping useful
>information into userspace. I'd like to either remove the variable or
>dump it to userspace in the same way the other variables are.

Thanks for reporting this.

The ops should actually be used, fixing the warning should be as simple
as adding this into mvpp2_dbgfs_prs_entry_init :

+       debugfs_create_file("pmap", 0444, prs_entry_dir, entry,
+                           &mvpp2_dbgfs_prs_pmap_fops);
+

>Wanted to reach out for opinions on the best course of action before
>submitting a patch.

Can you submit a patch, or do you prefer me to do it ?

Thanks,

Maxime

[PATCH] net: mvpp2: cls: Add pmap to fs dump

[Nathan Huckleberry]

There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
Added a usage consistent with other fops to dump pmap
to userspace.

Cc: clang-built-linux@googlegroups.com
Link: https://github.com/ClangBuiltLinux/linux/issues/529
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
 drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 0ee39ea47b6b..55947bc63cfd 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -566,6 +566,9 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
 	debugfs_create_file("hits", 0444, prs_entry_dir, entry,
 			    &mvpp2_dbgfs_prs_hits_fops);
 
+	ddebugfs_create_file("pmap", 0444, prs_entry_dir, entry,
+			     &mvpp2_dbgfs_prs_pmap_fops);
+
 	return 0;
 }
 
-- 
2.22.0.410.gd8fdbe21b5-goog

Re: [PATCH] net: mvpp2: cls: Add pmap to fs dump

[Nick Desaulniers]

On Tue, Jun 18, 2019 at 9:09 AM 'Nathan Huckleberry' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
> Added a usage consistent with other fops to dump pmap
> to userspace.
>
> Cc: clang-built-linux@googlegroups.com
> Link: https://github.com/ClangBuiltLinux/linux/issues/529
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>

Looks good to me based on Maxime's suggestion.  Thanks for seeking
clarification and following up on the feedback.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Nathan, you should use Suggested-by tags (liberally, IMO) when your
patch is based on feedback from others, in this case:
Suggested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

> ---
>  drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> index 0ee39ea47b6b..55947bc63cfd 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> @@ -566,6 +566,9 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
>         debugfs_create_file("hits", 0444, prs_entry_dir, entry,
>                             &mvpp2_dbgfs_prs_hits_fops);
>
> +       ddebugfs_create_file("pmap", 0444, prs_entry_dir, entry,
> +                            &mvpp2_dbgfs_prs_pmap_fops);
> +

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] net: mvpp2: cls: Add pmap to fs dump

[David Miller]

From: Nathan Huckleberry <nhuck@google.com>
Date: Tue, 18 Jun 2019 09:09:10 -0700

> +	ddebugfs_create_file("pmap", 0444, prs_entry_dir, entry,

drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c: In function ‘mvpp2_dbgfs_prs_entry_init’:
drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c:569:2: error: implicit declaration of function ‘ddebugfs_create_file’; did you mean ‘debugfs_create_file’? [-Werror=implicit-function-declaration]

This doesn't compile, did you build test this?

Re: [PATCH] net: mvpp2: cls: Add pmap to fs dump

[Maxime Chevallier]

Hello Nathan,

On Tue, 18 Jun 2019 09:09:10 -0700
Nathan Huckleberry <nhuck@google.com> wrote:

>There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
>Added a usage consistent with other fops to dump pmap
>to userspace.

Thanks for sending a fix. Besides the typo preventing your patch from
compiling, you should also prefix the patch by "net: mvpp2: debugfs:"
rather than "cls", which is used for classifier patches.

Thanks,

Maxime

[PATCH v2] net: mvpp2: debugfs: Add pmap to fs dump

[Nathan Huckleberry]

There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
Added a usage consistent with other fops to dump pmap
to userspace.

Cc: clang-built-linux@googlegroups.com
Link: https://github.com/ClangBuiltLinux/linux/issues/529
Signed-off-by: Nathan Huckleberry <nhuck@google.com>
---
Changes from v1 -> v2
* Fix typo
* Change commit prefix to debugfs
 drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
index 0ee39ea47b6b..274fb07362cb 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
@@ -566,6 +566,9 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
 	debugfs_create_file("hits", 0444, prs_entry_dir, entry,
 			    &mvpp2_dbgfs_prs_hits_fops);
 
+	debugfs_create_file("pmap", 0444, prs_entry_dir, entry,
+			     &mvpp2_dbgfs_prs_pmap_fops);
+
 	return 0;
 }
 
-- 
2.22.0.410.gd8fdbe21b5-goog

Re: [PATCH v2] net: mvpp2: debugfs: Add pmap to fs dump

[Nick Desaulniers]

On Wed, Jun 19, 2019 at 11:17 AM 'Nathan Huckleberry' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
> Added a usage consistent with other fops to dump pmap
> to userspace.

> Changes from v1 -> v2
> * Fix typo
> * Change commit prefix to debugfs

Compile-
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Requires `make ... W=1` before the patch to observe the warning.

>  drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> index 0ee39ea47b6b..274fb07362cb 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_debugfs.c
> @@ -566,6 +566,9 @@ static int mvpp2_dbgfs_prs_entry_init(struct dentry *parent,
>         debugfs_create_file("hits", 0444, prs_entry_dir, entry,
>                             &mvpp2_dbgfs_prs_hits_fops);
>
> +       debugfs_create_file("pmap", 0444, prs_entry_dir, entry,
> +                            &mvpp2_dbgfs_prs_pmap_fops);
> +

Thanks,
~Nick Desaulniers

Re: [PATCH v2] net: mvpp2: debugfs: Add pmap to fs dump

[David Miller]

From: Nathan Huckleberry <nhuck@google.com>
Date: Wed, 19 Jun 2019 11:17:15 -0700

> There was an unused variable 'mvpp2_dbgfs_prs_pmap_fops'
> Added a usage consistent with other fops to dump pmap
> to userspace.
> 
> Cc: clang-built-linux@googlegroups.com
> Link: https://github.com/ClangBuiltLinux/linux/issues/529
> Signed-off-by: Nathan Huckleberry <nhuck@google.com>

Applied.