[PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Qi Zheng]

Since commit ff9fb72bc077 ("debugfs: return error values, not NULL") changed
return value of debugfs_rename() in error cases from %NULL to %ERR_PTR(-ERROR).
The comments and checks corresponding to debugfs_rename() should also be updated
and fixed.

Qi Zheng (3):
  debugfs: update comment of debugfs_rename()
  bonding: fix error checking in bond_debug_reregister()
  PM/OPP: fix error checking in opp_migrate_dentry()

 drivers/net/bonding/bond_debugfs.c | 2 +-
 drivers/opp/debugfs.c              | 2 +-
 fs/debugfs/inode.c                 | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

-- 
2.20.1

[PATCH 1/3] debugfs: update comment of debugfs_rename()

[Qi Zheng]

Since commit ff9fb72bc077 ("debugfs: return error values,
not NULL") changed return value of debugfs_rename() in
error cases from %NULL to %ERR_PTR(-ERROR), the comment
of debugfs_rename should also be updated so as not to
mislead readers.

Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 fs/debugfs/inode.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 2e8e112b1993..58a35afb7c5d 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -802,8 +802,8 @@ EXPORT_SYMBOL_GPL(debugfs_lookup_and_remove);
  * exist for rename to succeed.
  *
  * This function will return a pointer to old_dentry (which is updated to
- * reflect renaming) if it succeeds. If an error occurs, %NULL will be
- * returned.
+ * reflect renaming) if it succeeds. If an error occurs, %ERR_PTR(-ERROR)
+ * will be returned.
  *
  * If debugfs is not enabled in the kernel, the value -%ENODEV will be
  * returned.
-- 
2.20.1

[PATCH 2/3] bonding: fix error checking in bond_debug_reregister()

[Qi Zheng]

Since commit ff9fb72bc077 ("debugfs: return error values,
not NULL") changed return value of debugfs_rename() in
error cases from %NULL to %ERR_PTR(-ERROR), we should
also check error values instead of NULL.

Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL")
Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 drivers/net/bonding/bond_debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 4f9b4a18c74c..594094526648 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -76,7 +76,7 @@ void bond_debug_reregister(struct bonding *bond)
 
 	d = debugfs_rename(bonding_debug_root, bond->debug_dir,
 			   bonding_debug_root, bond->dev->name);
-	if (d) {
+	if (!IS_ERR(d)) {
 		bond->debug_dir = d;
 	} else {
 		netdev_warn(bond->dev, "failed to reregister, so just unregister old one\n");
-- 
2.20.1

Re: [PATCH 2/3] bonding: fix error checking in bond_debug_reregister()

[Jay Vosburgh]

Qi Zheng <zhengqi.arch@bytedance.com> wrote:

>Since commit ff9fb72bc077 ("debugfs: return error values,
>not NULL") changed return value of debugfs_rename() in
>error cases from %NULL to %ERR_PTR(-ERROR), we should
>also check error values instead of NULL.
>
>Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL")
>Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>

Acked-by: Jay Vosburgh <jay.vosburgh@canonical.com>

>---
> drivers/net/bonding/bond_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
>index 4f9b4a18c74c..594094526648 100644
>--- a/drivers/net/bonding/bond_debugfs.c
>+++ b/drivers/net/bonding/bond_debugfs.c
>@@ -76,7 +76,7 @@ void bond_debug_reregister(struct bonding *bond)
> 
> 	d = debugfs_rename(bonding_debug_root, bond->debug_dir,
> 			   bonding_debug_root, bond->dev->name);
>-	if (d) {
>+	if (!IS_ERR(d)) {
> 		bond->debug_dir = d;
> 	} else {
> 		netdev_warn(bond->dev, "failed to reregister, so just unregister old one\n");
>-- 
>2.20.1
>

[PATCH 3/3] PM/OPP: fix error checking in opp_migrate_dentry()

[Qi Zheng]

Since commit ff9fb72bc077 ("debugfs: return error values,
not NULL") changed return value of debugfs_rename() in
error cases from %NULL to %ERR_PTR(-ERROR), we should
also check error values instead of NULL.

Fixes: ff9fb72bc077 ("debugfs: return error values, not NULL")
Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
---
 drivers/opp/debugfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/opp/debugfs.c b/drivers/opp/debugfs.c
index 96a30a032c5f..2c7fb683441e 100644
--- a/drivers/opp/debugfs.c
+++ b/drivers/opp/debugfs.c
@@ -235,7 +235,7 @@ static void opp_migrate_dentry(struct opp_device *opp_dev,
 
 	dentry = debugfs_rename(rootdir, opp_dev->dentry, rootdir,
 				opp_table->dentry_name);
-	if (!dentry) {
+	if (IS_ERR(dentry)) {
 		dev_err(dev, "%s: Failed to rename link from: %s to %s\n",
 			__func__, dev_name(opp_dev->dev), dev_name(dev));
 		return;
-- 
2.20.1

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Thu,  2 Feb 2023 17:32:53 +0800 you wrote:
> Since commit ff9fb72bc077 ("debugfs: return error values, not NULL") changed
> return value of debugfs_rename() in error cases from %NULL to %ERR_PTR(-ERROR).
> The comments and checks corresponding to debugfs_rename() should also be updated
> and fixed.
> 
> Qi Zheng (3):
>   debugfs: update comment of debugfs_rename()
>   bonding: fix error checking in bond_debug_reregister()
>   PM/OPP: fix error checking in opp_migrate_dentry()
> 
> [...]

Here is the summary with links:
  - [1/3] debugfs: update comment of debugfs_rename()
    (no matching commit)
  - [2/3] bonding: fix error checking in bond_debug_reregister()
    https://git.kernel.org/netdev/net/c/cbe83191d40d
  - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
    (no matching commit)

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Qi Zheng]

On 2023/2/4 11:30, patchwork-bot+netdevbpf@kernel.org wrote:
> Hello:
> 
> This series was applied to netdev/net.git (master)
> by Jakub Kicinski <kuba@kernel.org>:
> 
> On Thu,  2 Feb 2023 17:32:53 +0800 you wrote:
>> Since commit ff9fb72bc077 ("debugfs: return error values, not NULL") changed
>> return value of debugfs_rename() in error cases from %NULL to %ERR_PTR(-ERROR).
>> The comments and checks corresponding to debugfs_rename() should also be updated
>> and fixed.
>>
>> Qi Zheng (3):
>>    debugfs: update comment of debugfs_rename()
>>    bonding: fix error checking in bond_debug_reregister()
>>    PM/OPP: fix error checking in opp_migrate_dentry()
>>
>> [...]
> 
> Here is the summary with links:
>    - [1/3] debugfs: update comment of debugfs_rename()
>      (no matching commit)
>    - [2/3] bonding: fix error checking in bond_debug_reregister()
>      https://git.kernel.org/netdev/net/c/cbe83191d40d
>    - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
>      (no matching commit)

Hi all,

Does "no matching commit" means that these two patches have not been
applied? And I did not see them in the linux-next branch.

If so, hi Greg, Can you help to review and apply these two patches
([1/3] and [3/3])?

Thanks,
Qi

> 
> You are awesome, thank you!

-- 
Thanks,
Qi

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Jakub Kicinski]

On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
> > Here is the summary with links:
> >    - [1/3] debugfs: update comment of debugfs_rename()
> >      (no matching commit)
> >    - [2/3] bonding: fix error checking in bond_debug_reregister()
> >      https://git.kernel.org/netdev/net/c/cbe83191d40d
> >    - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
> >      (no matching commit)  
> 
> Does "no matching commit" means that these two patches have not been
> applied? And I did not see them in the linux-next branch.

Correct, we took the networking patch to the networking tree.
You'd be better off not grouping patches from different subsystems
if there are no dependencies. Maintainers may get confused about
who's supposed to apply them, err on the side of caution and 
not apply anything.

> If so, hi Greg, Can you help to review and apply these two patches
> ([1/3] and [3/3])?

Or 3/3 should go to Viresh?.. Dunno..

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Qi Zheng]

On 2023/2/8 02:31, Jakub Kicinski wrote:
> On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
>>> Here is the summary with links:
>>>     - [1/3] debugfs: update comment of debugfs_rename()
>>>       (no matching commit)
>>>     - [2/3] bonding: fix error checking in bond_debug_reregister()
>>>       https://git.kernel.org/netdev/net/c/cbe83191d40d
>>>     - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
>>>       (no matching commit)
>>
>> Does "no matching commit" means that these two patches have not been
>> applied? And I did not see them in the linux-next branch.
> 

Hi Jakub,

> Correct, we took the networking patch to the networking tree.

Thank you very much. :)

> You'd be better off not grouping patches from different subsystems
> if there are no dependencies. Maintainers may get confused about
> who's supposed to apply them, err on the side of caution and
> not apply anything.

Got it. I will send [1/3] and [3/3] separately.

> 
>> If so, hi Greg, Can you help to review and apply these two patches
>> ([1/3] and [3/3])?
> 
> Or 3/3 should go to Viresh?.. Dunno..

Got it.

Thanks,
Qi

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Greg Kroah-Hartman]

On Tue, Feb 07, 2023 at 10:31:24AM -0800, Jakub Kicinski wrote:
> On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
> > > Here is the summary with links:
> > >    - [1/3] debugfs: update comment of debugfs_rename()
> > >      (no matching commit)
> > >    - [2/3] bonding: fix error checking in bond_debug_reregister()
> > >      https://git.kernel.org/netdev/net/c/cbe83191d40d
> > >    - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
> > >      (no matching commit)  
> > 
> > Does "no matching commit" means that these two patches have not been
> > applied? And I did not see them in the linux-next branch.
> 
> Correct, we took the networking patch to the networking tree.
> You'd be better off not grouping patches from different subsystems
> if there are no dependencies. Maintainers may get confused about
> who's supposed to apply them, err on the side of caution and 
> not apply anything.
> 
> > If so, hi Greg, Can you help to review and apply these two patches
> > ([1/3] and [3/3])?

If someone sends me patch 1, I can and will review it then.  Otherwise,
digging it out of a random patch series is pretty impossible with my
patch load, sorry.

thanks,

greg k-h

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Qi Zheng]

On 2023/2/8 19:54, Greg Kroah-Hartman wrote:
> On Tue, Feb 07, 2023 at 10:31:24AM -0800, Jakub Kicinski wrote:
>> On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
>>>> Here is the summary with links:
>>>>     - [1/3] debugfs: update comment of debugfs_rename()
>>>>       (no matching commit)
>>>>     - [2/3] bonding: fix error checking in bond_debug_reregister()
>>>>       https://git.kernel.org/netdev/net/c/cbe83191d40d
>>>>     - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
>>>>       (no matching commit)
>>>
>>> Does "no matching commit" means that these two patches have not been
>>> applied? And I did not see them in the linux-next branch.
>>
>> Correct, we took the networking patch to the networking tree.
>> You'd be better off not grouping patches from different subsystems
>> if there are no dependencies. Maintainers may get confused about
>> who's supposed to apply them, err on the side of caution and
>> not apply anything.
>>
>>> If so, hi Greg, Can you help to review and apply these two patches
>>> ([1/3] and [3/3])?
> 
> If someone sends me patch 1, I can and will review it then.  Otherwise,
> digging it out of a random patch series is pretty impossible with my
> patch load, sorry.

Hi Greg,

Sorry about this. My bad. And I have sent the [1/3] separately, please 
review it if you have time. :)

Thanks,
Qi

> 
> thanks,
> 
> greg k-h

-- 
Thanks,
Qi

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Greg Kroah-Hartman]

On Wed, Feb 08, 2023 at 08:05:44PM +0800, Qi Zheng wrote:
> 
> 
> On 2023/2/8 19:54, Greg Kroah-Hartman wrote:
> > On Tue, Feb 07, 2023 at 10:31:24AM -0800, Jakub Kicinski wrote:
> > > On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
> > > > > Here is the summary with links:
> > > > >     - [1/3] debugfs: update comment of debugfs_rename()
> > > > >       (no matching commit)
> > > > >     - [2/3] bonding: fix error checking in bond_debug_reregister()
> > > > >       https://git.kernel.org/netdev/net/c/cbe83191d40d
> > > > >     - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
> > > > >       (no matching commit)
> > > > 
> > > > Does "no matching commit" means that these two patches have not been
> > > > applied? And I did not see them in the linux-next branch.
> > > 
> > > Correct, we took the networking patch to the networking tree.
> > > You'd be better off not grouping patches from different subsystems
> > > if there are no dependencies. Maintainers may get confused about
> > > who's supposed to apply them, err on the side of caution and
> > > not apply anything.
> > > 
> > > > If so, hi Greg, Can you help to review and apply these two patches
> > > > ([1/3] and [3/3])?
> > 
> > If someone sends me patch 1, I can and will review it then.  Otherwise,
> > digging it out of a random patch series is pretty impossible with my
> > patch load, sorry.
> 
> Hi Greg,
> 
> Sorry about this. My bad. And I have sent the [1/3] separately, please
> review it if you have time. :)

Ick, somehow all of these got marked as spam by my filters.  I'll look
at them next week, sorry for the delay.

greg k-h

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Qi Zheng]

On 2023/2/11 17:06, Greg Kroah-Hartman wrote:
> On Wed, Feb 08, 2023 at 08:05:44PM +0800, Qi Zheng wrote:
>>
>>
>> On 2023/2/8 19:54, Greg Kroah-Hartman wrote:
>>> On Tue, Feb 07, 2023 at 10:31:24AM -0800, Jakub Kicinski wrote:
>>>> On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
>>>>>> Here is the summary with links:
>>>>>>      - [1/3] debugfs: update comment of debugfs_rename()
>>>>>>        (no matching commit)
>>>>>>      - [2/3] bonding: fix error checking in bond_debug_reregister()
>>>>>>        https://git.kernel.org/netdev/net/c/cbe83191d40d
>>>>>>      - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
>>>>>>        (no matching commit)
>>>>>
>>>>> Does "no matching commit" means that these two patches have not been
>>>>> applied? And I did not see them in the linux-next branch.
>>>>
>>>> Correct, we took the networking patch to the networking tree.
>>>> You'd be better off not grouping patches from different subsystems
>>>> if there are no dependencies. Maintainers may get confused about
>>>> who's supposed to apply them, err on the side of caution and
>>>> not apply anything.
>>>>
>>>>> If so, hi Greg, Can you help to review and apply these two patches
>>>>> ([1/3] and [3/3])?
>>>
>>> If someone sends me patch 1, I can and will review it then.  Otherwise,
>>> digging it out of a random patch series is pretty impossible with my
>>> patch load, sorry.
>>
>> Hi Greg,
>>
>> Sorry about this. My bad. And I have sent the [1/3] separately, please
>> review it if you have time. :)
> 
> Ick, somehow all of these got marked as spam by my filters.  I'll look
> at them next week, sorry for the delay.
No worries. And the patch link that has been resent is
https://lore.kernel.org/lkml/20230208035634.58095-1-zhengqi.arch@bytedance.com/. 
:)

Thank you very much,
Qi

> 
> greg k-h

Re: [PATCH 0/3] some minor fixes of error checking about debugfs_rename()

[Greg Kroah-Hartman]

On Sat, Feb 11, 2023 at 05:12:46PM +0800, Qi Zheng wrote:
> 
> 
> On 2023/2/11 17:06, Greg Kroah-Hartman wrote:
> > On Wed, Feb 08, 2023 at 08:05:44PM +0800, Qi Zheng wrote:
> > > 
> > > 
> > > On 2023/2/8 19:54, Greg Kroah-Hartman wrote:
> > > > On Tue, Feb 07, 2023 at 10:31:24AM -0800, Jakub Kicinski wrote:
> > > > > On Tue, 7 Feb 2023 18:30:40 +0800 Qi Zheng wrote:
> > > > > > > Here is the summary with links:
> > > > > > >      - [1/3] debugfs: update comment of debugfs_rename()
> > > > > > >        (no matching commit)
> > > > > > >      - [2/3] bonding: fix error checking in bond_debug_reregister()
> > > > > > >        https://git.kernel.org/netdev/net/c/cbe83191d40d
> > > > > > >      - [3/3] PM/OPP: fix error checking in opp_migrate_dentry()
> > > > > > >        (no matching commit)
> > > > > > 
> > > > > > Does "no matching commit" means that these two patches have not been
> > > > > > applied? And I did not see them in the linux-next branch.
> > > > > 
> > > > > Correct, we took the networking patch to the networking tree.
> > > > > You'd be better off not grouping patches from different subsystems
> > > > > if there are no dependencies. Maintainers may get confused about
> > > > > who's supposed to apply them, err on the side of caution and
> > > > > not apply anything.
> > > > > 
> > > > > > If so, hi Greg, Can you help to review and apply these two patches
> > > > > > ([1/3] and [3/3])?
> > > > 
> > > > If someone sends me patch 1, I can and will review it then.  Otherwise,
> > > > digging it out of a random patch series is pretty impossible with my
> > > > patch load, sorry.
> > > 
> > > Hi Greg,
> > > 
> > > Sorry about this. My bad. And I have sent the [1/3] separately, please
> > > review it if you have time. :)
> > 
> > Ick, somehow all of these got marked as spam by my filters.  I'll look
> > at them next week, sorry for the delay.
> No worries. And the patch link that has been resent is
> https://lore.kernel.org/lkml/20230208035634.58095-1-zhengqi.arch@bytedance.com/.
> :)

Both now queued up, thanks.

greg k-h