diff mbox series

ubifs: remove unnecessary kobject_del()

Message ID 20230405130747.66006-1-frank.li@vivo.com
State New
Headers
Series ubifs: remove unnecessary kobject_del() |

Commit Message

李扬韬 April 5, 2023, 1:07 p.m. UTC 
  kobject_put() actually covers kobject removal automatically, which is
single stage removal. So it is safe to kill kobject_del() directly.

Signed-off-by: Yangtao Li <frank.li@vivo.com>
---
 fs/ubifs/sysfs.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Zhihao Cheng April 6, 2023, 3:36 a.m. UTC | #1 
Hi, Yangtao
> kobject_put() actually covers kobject removal automatically, which is
> single stage removal. So it is safe to kill kobject_del() directly.
> 
> Signed-off-by: Yangtao Li <frank.li@vivo.com>
> ---
>   fs/ubifs/sysfs.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
> index 1c958148bb87..1ffdc3c9b340 100644
> --- a/fs/ubifs/sysfs.c
> +++ b/fs/ubifs/sysfs.c
> @@ -130,7 +130,6 @@ int ubifs_sysfs_register(struct ubifs_info *c)
>   
>   void ubifs_sysfs_unregister(struct ubifs_info *c)
>   {
> -	kobject_del(&c->kobj);
>   	kobject_put(&c->kobj);
>   	wait_for_completion(&c->kobj_unregister);
>   
> 

This patch looks harmless at the view of ubifs, kobject_cleanup() 
finally invokes __kobject_del and releases parent just like 
kobject_del() does. A difference is that the releasing sequence of 
kobj's parent is put after releasing kobj.

About the use case of kobject_del(), we may refer to other filesystems' 
implementations, eg. ext4_put_super(). Firstly ext4_unregister_sysfs() 
removes all sysfs interfaces, then jbd2_journal_destroy() destroys 
sbi->s_journal, finally sbi->s_kobj is released. Here kobject_del() 
stops user accessing sbi->s_journal by sysfs interface 
journal_task，because sbi->s_journal will be released soon before 
kobject_put().

I think we should still reserve the 'redundant' kobject_del(), removing 
it won't bring any performance improvement.

BTW, in ext4_put_super(), flush_stashed_error_work(which could access 
sbi->s_kobj) is flushed after kobject removed. is it okay to replace 
kobject_del(&sbi->s_kobj) with kobject_put(&sbi->s_kobj)?

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index a22417d113ca..9e3744099d1e 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -1296,8 +1296,6 @@ static void ext4_put_super(struct super_block *sb)
          * Now that we are completely done shutting down the
          * superblock, we need to actually destroy the kobject.
          */
-       kobject_put(&sbi->s_kobj);
-       wait_for_completion(&sbi->s_kobj_unregister);
         if (sbi->s_chksum_driver)
                 crypto_free_shash(sbi->s_chksum_driver);
         kfree(sbi->s_blockgroup_lock);
diff --git a/fs/ext4/sysfs.c b/fs/ext4/sysfs.c
index 12d6252e3e22..be92a09bb8a0 100644
--- a/fs/ext4/sysfs.c
+++ b/fs/ext4/sysfs.c
@@ -562,7 +562,8 @@ void ext4_unregister_sysfs(struct super_block *sb)

         if (sbi->s_proc)
                 remove_proc_subtree(sb->s_id, ext4_proc_root);
-       kobject_del(&sbi->s_kobj);
+       kobject_put(&sbi->s_kobj);
+       wait_for_completion(&sbi->s_kobj_unregister);
  }

  int __init ext4_init_sysfs(void)
李扬韬 April 6, 2023, 9:08 a.m. UTC | #2 
> I think we should still reserve the 'redundant' kobject_del(),
> removing it won't bring any performance improvement.

Since it's redundant, why not to remove it.

Thx,
Yangtao
Zhihao Cheng April 6, 2023, 12:29 p.m. UTC | #3 
Hi,
>> I think we should still reserve the 'redundant' kobject_del(),
>> removing it won't bring any performance improvement.
> 
> Since it's redundant, why not to remove it.
> 

In my personal view, 'redundant' means removing kobject_del() is okay, 
it won't bring any bugs. But removing it won't make code more 
readability or gain any performance improvement, so it could be 
reserved. Whether to remove kobject_del() depends on UBIFS maintainer, I 
just help to check if the modification could affect the normal logic.
diff mbox series

Patch

diff --git a/fs/ubifs/sysfs.c b/fs/ubifs/sysfs.c
index 1c958148bb87..1ffdc3c9b340 100644
--- a/fs/ubifs/sysfs.c
+++ b/fs/ubifs/sysfs.c
@@ -130,7 +130,6 @@  int ubifs_sysfs_register(struct ubifs_info *c)
 
 void ubifs_sysfs_unregister(struct ubifs_info *c)
 {
-	kobject_del(&c->kobj);
 	kobject_put(&c->kobj);
 	wait_for_completion(&c->kobj_unregister);