[PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[Wang Ming]

It is expected that most callers should _ignore_ the errors
return by debugfs_create_dir() in bond_debug_reregister().

Signed-off-by: Wang Ming <machel@vivo.com>
---
 drivers/net/bonding/bond_debugfs.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 594094526648..a41f765420dc 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -87,9 +87,6 @@ void bond_debug_reregister(struct bonding *bond)
 void bond_create_debugfs(void)
 {
 	bonding_debug_root = debugfs_create_dir("bonding", NULL);
-
-	if (!bonding_debug_root)
-		pr_warn("Warning: Cannot create bonding directory in debugfs\n");
 }
 
 void bond_destroy_debugfs(void)
-- 
2.25.1

Re: [PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[Markus Elfring]

> It is expected that most callers should _ignore_ the errors
> return by debugfs_create_dir() in bond_debug_reregister().

Did you overlook any patch review comments once more anyhow?

Regards,
Markus

Re: [PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[Jay Vosburgh]

Wang Ming <machel@vivo.com> wrote:

>It is expected that most callers should _ignore_ the errors
>return by debugfs_create_dir() in bond_debug_reregister().

	Why should the error be ignored?  It's not a fatal error, in the
sense that the bond itself should be unregistered, but I'm not sure why
an error message that the debugfs registration failed is undesirable.

	Also, the code in question is in bond_create_debugfs(), not
bond_debug_reregister().  The diff below looks a bit odd in that the
context line lists _reregister, but that's not the function being
changed.

	I thought the v1 patch was fine.

	-J

>Signed-off-by: Wang Ming <machel@vivo.com>
>---
> drivers/net/bonding/bond_debugfs.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
>index 594094526648..a41f765420dc 100644
>--- a/drivers/net/bonding/bond_debugfs.c
>+++ b/drivers/net/bonding/bond_debugfs.c
>@@ -87,9 +87,6 @@ void bond_debug_reregister(struct bonding *bond)
> void bond_create_debugfs(void)
> {
> 	bonding_debug_root = debugfs_create_dir("bonding", NULL);
>-
>-	if (!bonding_debug_root)
>-		pr_warn("Warning: Cannot create bonding directory in debugfs\n");
> }
> 
> void bond_destroy_debugfs(void)
>-- 
>2.25.1
>

---
	-Jay Vosburgh, jay.vosburgh@canonical.com

Re: [PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[Markus Elfring]

> > It is expected that most callers should _ignore_ the errors
> > return by debugfs_create_dir() in bond_debug_reregister().
>
> 	Why should the error be ignored?  It's not a fatal error, in the
> sense that the bond itself should be unregistered, but I'm not sure why
> an error message that the debugfs registration failed is undesirable.

Would you like to insist on the possibility to get informed anyhow
about a bonding debugfs directory creation failure?
https://elixir.bootlin.com/linux/v6.5-rc1/source/drivers/net/bonding/bond_debugfs.c#L87


> 	Also, the code in question is in bond_create_debugfs(), not
> bond_debug_reregister().

Is it interesting how improvable change descriptions are presented?


>                           The diff below looks a bit odd in that the
> context line lists _reregister, but that's not the function being
> changed.

I do also not see the mentioned identifier in an update candidate.


> 	I thought the v1 patch was fine.

This change approach (from 2023-07-13) looked mostly appropriate.

[PATCH net v1] net:bonding:Fix error checking for debugfs_create_dir
https://lore.kernel.org/lkml/20230713033607.12804-1-machel@vivo.com/

Will the review attention grow for any remaining concerns or ideas?

Regards,
Markus

回复: [PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[王明-软件底层技术部]

Ok, I will amend v1 and resubmit v3.
Regards
Wang Ming

-----邮件原件-----
发件人: Jay Vosburgh <jay.vosburgh@canonical.com> 
发送时间: 2023年7月15日 0:12
收件人: 王明-软件底层技术部 <machel@vivo.com>
抄送: Andy Gospodarek <andy@greyhouse.net>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; opensource.kernel <opensource.kernel@vivo.com>
主题: Re: [PATCH net v2] net: bonding: Remove error checking for debugfs_create_dir()

[????????? jay.vosburgh@canonical.com ????????? https://aka.ms/LearnAboutSenderIdentification?????????????]

Wang Ming <machel@vivo.com> wrote:

>It is expected that most callers should _ignore_ the errors return by 
>debugfs_create_dir() in bond_debug_reregister().

        Why should the error be ignored?  It's not a fatal error, in the sense that the bond itself should be unregistered, but I'm not sure why an error message that the debugfs registration failed is undesirable.

        Also, the code in question is in bond_create_debugfs(), not bond_debug_reregister().  The diff below looks a bit odd in that the context line lists _reregister, but that's not the function being changed.

        I thought the v1 patch was fine.

        -J

>Signed-off-by: Wang Ming <machel@vivo.com>
>---
> drivers/net/bonding/bond_debugfs.c | 3 ---
> 1 file changed, 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_debugfs.c 
>b/drivers/net/bonding/bond_debugfs.c
>index 594094526648..a41f765420dc 100644
>--- a/drivers/net/bonding/bond_debugfs.c
>+++ b/drivers/net/bonding/bond_debugfs.c
>@@ -87,9 +87,6 @@ void bond_debug_reregister(struct bonding *bond)  
>void bond_create_debugfs(void)  {
>       bonding_debug_root = debugfs_create_dir("bonding", NULL);
>-
>-      if (!bonding_debug_root)
>-              pr_warn("Warning: Cannot create bonding directory in debugfs\n");
> }
>
> void bond_destroy_debugfs(void)
>--
>2.25.1
>

---
        -Jay Vosburgh, jay.vosburgh@canonical.com