[lustre-devel] [PATCH v2 05/29] lustre: llite: don't use class_setup_tunables()

[NeilBrown]

On Mon, May 20 2019, James Simmons wrote:

> llite is very different from the other types of lustre devices.
> Since this is the case llite should register independently. Doing
> this allows us to cleanup the debugfs registering in the release
> function of struct kobj_type.
>
> Signed-off-by: James Simmons <uja.ornl at yahoo.com>
> WC-bug-id: https://jira.whamcloud.com/browse/LU-8066
> Reviewed-on: https://review.whamcloud.com/34292
> Reviewed-by: Andreas Dilger <adilger at whamcloud.com>
> Reviewed-by: Ben Evans <bevans at cray.com>
> Reviewed-by: Oleg Drokin <green at whamcloud.com>
> Signed-off-by: James Simmons <jsimmons at infradead.org>
> ---
>  fs/lustre/llite/lproc_llite.c | 41 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 30 insertions(+), 11 deletions(-)
>
> diff --git a/fs/lustre/llite/lproc_llite.c b/fs/lustre/llite/lproc_llite.c
> index 5de8462..91b0a50 100644
> --- a/fs/lustre/llite/lproc_llite.c
> +++ b/fs/lustre/llite/lproc_llite.c
> @@ -42,18 +42,40 @@
>  static struct kobject *llite_kobj;
>  static struct dentry *llite_root;
>  
> +static void llite_kobj_release(struct kobject *kobj)
> +{
> +	if (!IS_ERR_OR_NULL(llite_root)) {
> +		debugfs_remove(llite_root);
> +		llite_root = NULL;
> +	}
> +
> +	kfree(kobj);
> +}
> +
> +static struct kobj_type llite_kobj_ktype = {
> +	.release	= llite_kobj_release,
> +	.sysfs_ops	= &lustre_sysfs_ops,
> +};
> +
>  int llite_tunables_register(void)
>  {
> -	int rc = 0;
> +	int rc;
> +
> +	llite_kobj = kzalloc(sizeof(*llite_kobj), GFP_KERNEL);
> +	if (!llite_kobj)
> +		return -ENOMEM;
>  
> -	llite_kobj = class_setup_tunables("llite");
> -	if (IS_ERR(llite_kobj))
> -		return PTR_ERR(llite_kobj);
> +	llite_kobj->kset = lustre_kset;
> +	rc = kobject_init_and_add(llite_kobj, &llite_kobj_ktype,
> +				  &lustre_kset->kobj, "%s", "llite");
> +	if (rc)
> +		goto free_kobj;
>  
>  	llite_root = debugfs_create_dir("llite", debugfs_lustre_root);
>  	if (IS_ERR_OR_NULL(llite_root)) {
>  		rc = llite_root ? PTR_ERR(llite_root) : -ENOMEM;
>  		llite_root = NULL;
> +free_kobj:
>  		kobject_put(llite_kobj);
>  		llite_kobj = NULL;

eeek.  Goto into the inside of an if/then clause is .... not my
favourite.

> +	rc = kobject_init_and_add(llite_kobj, &llite_kobj_ktype,
> +				  &lustre_kset->kobj, "%s", "llite");
> +	if (rc)
> +		goto free_kobj;
>  
>  	llite_root = debugfs_create_dir("llite", debugfs_lustre_root);
>  	if (IS_ERR_OR_NULL(llite_root)) {
>  		rc = llite_root ? PTR_ERR(llite_root) : -ENOMEM;
>  		llite_root = NULL;
                goto free_kobj;
        }
        return 0;
> +free_kobj:
> 	kobject_put(llite_kobj);
> 	llite_kobj = NULL;
        return rc;

Isn't that much cleaner?

Otherwise, I like the patch.

Thanks,
NeilBrown


>  	}
> @@ -65,9 +87,6 @@ void llite_tunables_unregister(void)
>  {
>  	kobject_put(llite_kobj);
>  	llite_kobj = NULL;
> -
> -	debugfs_remove(llite_root);
> -	llite_root = NULL;
>  }
>  
>  /* debugfs llite mount point registration */
> @@ -1216,17 +1235,17 @@ static ssize_t ll_nosquash_nids_seq_write(struct file *file,
>  	NULL,
>  };
>  
> -static void llite_kobj_release(struct kobject *kobj)
> +static void sbi_kobj_release(struct kobject *kobj)
>  {
>  	struct ll_sb_info *sbi = container_of(kobj, struct ll_sb_info,
>  					      ll_kset.kobj);
>  	complete(&sbi->ll_kobj_unregister);
>  }
>  
> -static struct kobj_type llite_ktype = {
> +static struct kobj_type sbi_ktype = {
>  	.default_attrs	= llite_attrs,
>  	.sysfs_ops	= &lustre_sysfs_ops,
> -	.release	= llite_kobj_release,
> +	.release	= sbi_kobj_release,
>  };
>  
>  static const struct llite_file_opcode {
> @@ -1384,7 +1403,7 @@ int ll_debugfs_register_super(struct super_block *sb, const char *name)
>  out_ll_kset:
>  	/* Yes we also register sysfs mount kset here as well */
>  	sbi->ll_kset.kobj.parent = llite_kobj;
> -	sbi->ll_kset.kobj.ktype = &llite_ktype;
> +	sbi->ll_kset.kobj.ktype = &sbi_ktype;
>  	init_completion(&sbi->ll_kobj_unregister);
>  	err = kobject_set_name(&sbi->ll_kset.kobj, "%s", name);
>  	if (err)
> -- 
> 1.8.3.1
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.lustre.org/pipermail/lustre-devel-lustre.org/attachments/20190522/757f908a/attachment.sig>