Re: [PATCH] ubifs: remove unnecessary kobject_del()

[Zhihao Cheng]

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@xxxxxxxx>
> ---
>   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)