From 392dab3c01bb802892466f9a05ef1b56406b8522 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 30 Apr 2019 11:09:25 -0400 Subject: [PATCH] LU-8066 obd: embed typ_kobj in obd_type As there is a 1-1 mapping between obd_types and their ->typ_kobj, it is simple and more normal to embed the kobj in the obd_type, rather than allocate it separately. This requires calling "kobject_init_and_add()" earlier, so we open-code relevant part of class_setup_tunables() in class_register_type(). Now class_setup_tunables() is needed only for server side code. With typ_kobj embedded in obd_type we change class_setup_tunables() to return an obd_type object instead of a kobject. This way we can use kobject_put() to cleanup the obd_type created with class_setup_tunables(). The reason for class_setup_tunables() is for the creation of a lightweight obd_type which is never added to the typ_chain list to avoid potential duplicates which can happen on single node setups with lod / lov and osp /osc. Change-Id: Iac160e6817a7c520e4462a3fc133ddfee6a7ccdc Signed-off-by: NeilBrown Signed-off-by: James Simmons Reviewed-on: https://review.whamcloud.com/34612 Reviewed-by: Ben Evans Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Oleg Drokin --- lustre/include/obd.h | 2 +- lustre/include/obd_class.h | 4 +- lustre/lod/lod_dev.c | 35 ++++++------ lustre/mdd/mdd_lproc.c | 2 +- lustre/obdclass/dt_object.c | 2 +- lustre/obdclass/genops.c | 112 +++++++++++++++++++-------------------- lustre/obdclass/lprocfs_status.c | 2 +- lustre/osp/osp_dev.c | 35 ++++++------ 8 files changed, 93 insertions(+), 101 deletions(-) diff --git a/lustre/include/obd.h b/lustre/include/obd.h index 44c6ced..4466d63 100644 --- a/lustre/include/obd.h +++ b/lustre/include/obd.h @@ -110,7 +110,7 @@ struct obd_type { int typ_refcnt; struct lu_device_type *typ_lu; spinlock_t obd_type_lock; - struct kobject *typ_kobj; + struct kobject typ_kobj; }; struct brw_page { diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index d0e4a0d..359e7ef 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -63,7 +63,9 @@ struct lu_device_type; /* genops.c */ struct obd_export *class_conn2export(struct lustre_handle *); -struct kobject *class_setup_tunables(const char *name); +#ifdef HAVE_SERVER_SUPPORT +struct obd_type *class_setup_tunables(const char *name); +#endif int class_register_type(struct obd_ops *, struct md_ops *, bool enable_proc, struct lprocfs_vars *module_vars, const char *nm, struct lu_device_type *ldt); diff --git a/lustre/lod/lod_dev.c b/lustre/lod/lod_dev.c index 96e0e2a..8285524 100644 --- a/lustre/lod/lod_dev.c +++ b/lustre/lod/lod_dev.c @@ -2223,13 +2223,12 @@ static struct obd_ops lod_obd_device_ops = { .o_pool_del = lod_pool_del, }; -static struct obd_type sym; +static struct obd_type *sym; static int __init lod_init(void) { struct dentry *symlink; struct obd_type *type; - struct kobject *kobj; struct qstr dname; int rc; @@ -2244,6 +2243,15 @@ static int __init lod_init(void) return rc; } + sym = class_setup_tunables(LUSTRE_LOV_NAME); + if (IS_ERR(sym)) { + rc = PTR_ERR(sym); + /* does real "lov" already exist ? */ + if (rc == -EEXIST) + GOTO(try_proc, rc = 0); + GOTO(no_lov, rc); + } + /* create "lov" entry for compatibility purposes */ dname.name = "lov"; dname.len = strlen(dname.name); @@ -2256,26 +2264,11 @@ static int __init lod_init(void) rc = symlink ? PTR_ERR(symlink) : -ENOMEM; GOTO(no_lov, rc); } - sym.typ_debugfs_entry = symlink; + sym->typ_debugfs_entry = symlink; } else { dput(symlink); } - kobj = kset_find_obj(lustre_kset, dname.name); - if (kobj) { - kobject_put(kobj); - goto try_proc; - } - - kobj = class_setup_tunables(dname.name); - if (IS_ERR(kobj)) { - rc = PTR_ERR(kobj); - if (sym.typ_debugfs_entry) - ldebugfs_remove(&sym.typ_debugfs_entry); - GOTO(no_lov, rc); - } - sym.typ_kobj = kobj; - try_proc: type = class_search_type(LUSTRE_LOV_NAME); if (type && type->typ_procroot) @@ -2295,8 +2288,10 @@ no_lov: static void __exit lod_exit(void) { - ldebugfs_remove(&sym.typ_debugfs_entry); - kobject_put(sym.typ_kobj); + if (!IS_ERR_OR_NULL(sym)) { + ldebugfs_remove(&sym->typ_debugfs_entry); + kobject_put(&sym->typ_kobj); + } class_unregister_type(LUSTRE_LOD_NAME); lu_kmem_fini(lod_caches); } diff --git a/lustre/mdd/mdd_lproc.c b/lustre/mdd/mdd_lproc.c index e53283b..d17d3c9 100644 --- a/lustre/mdd/mdd_lproc.c +++ b/lustre/mdd/mdd_lproc.c @@ -590,7 +590,7 @@ int mdd_procfs_init(struct mdd_device *mdd, const char *name) init_completion(&mdd->mdd_kobj_unregister); rc = kobject_init_and_add(&mdd->mdd_kobj, &mdd->mdd_ktype, - type->typ_kobj, "%s", name); + &type->typ_kobj, "%s", name); if (rc) return rc; diff --git a/lustre/obdclass/dt_object.c b/lustre/obdclass/dt_object.c index 9cacbe8..5cea5a6 100644 --- a/lustre/obdclass/dt_object.c +++ b/lustre/obdclass/dt_object.c @@ -1265,7 +1265,7 @@ int dt_tunables_init(struct dt_device *dt, struct obd_type *type, dt->dd_ktype.release = dt_sysfs_release; init_completion(&dt->dd_kobj_unregister); - rc = kobject_init_and_add(&dt->dd_kobj, &dt->dd_ktype, type->typ_kobj, + rc = kobject_init_and_add(&dt->dd_kobj, &dt->dd_ktype, &type->typ_kobj, "%s", name); if (rc) return rc; diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index 0a840c9..9508495 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -160,7 +160,9 @@ void class_put_type(struct obd_type *type) static void class_sysfs_release(struct kobject *kobj) { - OBD_FREE(kobj, sizeof(*kobj)); + struct obd_type *type = container_of(kobj, struct obd_type, typ_kobj); + + OBD_FREE(type, sizeof(*type)); } static struct kobj_type class_ktype = { @@ -168,30 +170,34 @@ static struct kobj_type class_ktype = { .release = class_sysfs_release, }; -struct kobject *class_setup_tunables(const char *name) +#ifdef HAVE_SERVER_SUPPORT +struct obd_type *class_setup_tunables(const char *name) { + struct obd_type *type; struct kobject *kobj; int rc; -#ifdef HAVE_SERVER_SUPPORT kobj = kset_find_obj(lustre_kset, name); - if (kobj) - return kobj; -#endif - OBD_ALLOC(kobj, sizeof(*kobj)); - if (!kobj) + if (kobj) { + kobject_put(kobj); + return ERR_PTR(-EEXIST); + } + + OBD_ALLOC(type, sizeof(*type)); + if (!type) return ERR_PTR(-ENOMEM); - kobj->kset = lustre_kset; - kobject_init(kobj, &class_ktype); - rc = kobject_add(kobj, &lustre_kset->kobj, "%s", name); + type->typ_kobj.kset = lustre_kset; + kobject_init(&type->typ_kobj, &class_ktype); + rc = kobject_add(&type->typ_kobj, &lustre_kset->kobj, "%s", name); if (rc) { - kobject_put(kobj); + kobject_put(&type->typ_kobj); return ERR_PTR(rc); } - return kobj; + return type; } EXPORT_SYMBOL(class_setup_tunables); +#endif /* HAVE_SERVER_SUPPORT */ #define CLASS_MAX_NAME 1024 @@ -201,9 +207,9 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, { struct obd_type *type; #ifdef HAVE_SERVER_SUPPORT - struct qstr dname; + struct kobject *kobj; #endif /* HAVE_SERVER_SUPPORT */ - int rc = 0; + int rc; ENTRY; /* sanity check */ @@ -214,11 +220,36 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, RETURN(-EEXIST); } - rc = -ENOMEM; +#ifdef HAVE_SERVER_SUPPORT + kobj = kset_find_obj(lustre_kset, name); + if (kobj) { + type = container_of(kobj, struct obd_type, typ_kobj); + + goto dir_exist; + } +#endif /* HAVE_SERVER_SUPPORT */ + OBD_ALLOC(type, sizeof(*type)); if (type == NULL) - RETURN(rc); + RETURN(-ENOMEM); + + type->typ_kobj.kset = lustre_kset; + rc = kobject_init_and_add(&type->typ_kobj, &class_ktype, + &lustre_kset->kobj, "%s", name); + if (rc) + GOTO(failed, rc); + type->typ_debugfs_entry = ldebugfs_register(name, debugfs_lustre_root, + vars, type); + if (IS_ERR_OR_NULL(type->typ_debugfs_entry)) { + rc = type->typ_debugfs_entry ? PTR_ERR(type->typ_debugfs_entry) + : -ENOMEM; + type->typ_debugfs_entry = NULL; + GOTO(failed, rc); + } +#ifdef HAVE_SERVER_SUPPORT +dir_exist: +#endif /* HAVE_SERVER_SUPPORT */ OBD_ALLOC_PTR(type->typ_dt_ops); OBD_ALLOC_PTR(type->typ_md_ops); OBD_ALLOC(type->typ_name, strlen(name) + 1); @@ -236,7 +267,7 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, spin_lock_init(&type->obd_type_lock); #ifdef CONFIG_PROC_FS - if (enable_proc) { + if (enable_proc && !type->typ_procroot) { type->typ_procroot = lprocfs_register(type->typ_name, proc_lustre_root, NULL, type); @@ -247,42 +278,11 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, } } #endif -#ifdef HAVE_SERVER_SUPPORT - dname.name = name; - dname.len = strlen(dname.name); - dname.hash = ll_full_name_hash(debugfs_lustre_root, dname.name, - dname.len); - type->typ_debugfs_entry = d_lookup(debugfs_lustre_root, &dname); - if (type->typ_debugfs_entry) { - dput(type->typ_debugfs_entry); - type->typ_sym_filter = true; - goto dir_exist; - } -#endif /* HAVE_SERVER_SUPPORT */ - - type->typ_debugfs_entry = ldebugfs_register(type->typ_name, - debugfs_lustre_root, - vars, type); - if (IS_ERR_OR_NULL(type->typ_debugfs_entry)) { - rc = type->typ_debugfs_entry ? PTR_ERR(type->typ_debugfs_entry) - : -ENOMEM; - type->typ_debugfs_entry = NULL; - GOTO(failed, rc); - } -#ifdef HAVE_SERVER_SUPPORT -dir_exist: -#endif - type->typ_kobj = class_setup_tunables(type->typ_name); - if (IS_ERR(type->typ_kobj)) - GOTO(failed, rc = PTR_ERR(type->typ_kobj)); - if (ldt) { type->typ_lu = ldt; rc = lu_device_type_init(ldt); - if (rc) { - kobject_put(type->typ_kobj); + if (rc) GOTO(failed, rc); - } } spin_lock(&obd_types_lock); @@ -309,8 +309,9 @@ failed: OBD_FREE_PTR(type->typ_md_ops); if (type->typ_dt_ops != NULL) OBD_FREE_PTR(type->typ_dt_ops); - OBD_FREE(type, sizeof(*type)); - RETURN(rc); + kobject_put(&type->typ_kobj); + + RETURN(rc); } EXPORT_SYMBOL(class_register_type); @@ -333,8 +334,6 @@ int class_unregister_type(const char *name) RETURN(-EBUSY); } - kobject_put(type->typ_kobj); - /* we do not use type->typ_procroot as for compatibility purposes * other modules can share names (i.e. lod can use lov entry). so * we can't reference pointer as it can get invalided when another @@ -363,8 +362,9 @@ int class_unregister_type(const char *name) OBD_FREE_PTR(type->typ_dt_ops); if (type->typ_md_ops != NULL) OBD_FREE_PTR(type->typ_md_ops); - OBD_FREE(type, sizeof(*type)); - RETURN(0); + kobject_put(&type->typ_kobj); + + RETURN(0); } /* class_unregister_type */ EXPORT_SYMBOL(class_unregister_type); diff --git a/lustre/obdclass/lprocfs_status.c b/lustre/obdclass/lprocfs_status.c index aa2925a..6c5d61f 100644 --- a/lustre/obdclass/lprocfs_status.c +++ b/lustre/obdclass/lprocfs_status.c @@ -1204,7 +1204,7 @@ int lprocfs_obd_setup(struct obd_device *obd, bool uuid_only) obd->obd_ktype.sysfs_ops = &lustre_sysfs_ops; obd->obd_ktype.release = obd_sysfs_release; - obd->obd_kset.kobj.parent = obd->obd_type->typ_kobj; + obd->obd_kset.kobj.parent = &obd->obd_type->typ_kobj; obd->obd_kset.kobj.ktype = &obd->obd_ktype; init_completion(&obd->obd_kobj_unregister); rc = kset_register(&obd->obd_kset); diff --git a/lustre/osp/osp_dev.c b/lustre/osp/osp_dev.c index b6e88ce..0aaf162 100644 --- a/lustre/osp/osp_dev.c +++ b/lustre/osp/osp_dev.c @@ -1881,7 +1881,7 @@ static struct obd_ops osp_obd_device_ops = { .o_fid_alloc = osp_fid_alloc, }; -static struct obd_type sym; +static struct obd_type *sym; /** * Initialize OSP module. @@ -1899,7 +1899,6 @@ static int __init osp_init(void) { struct dentry *symlink; struct obd_type *type; - struct kobject *kobj; struct qstr dname; int rc; @@ -1922,6 +1921,15 @@ static int __init osp_init(void) return rc; } + sym = class_setup_tunables(LUSTRE_OSC_NAME); + if (IS_ERR(sym)) { + rc = PTR_ERR(sym); + /* does real "osc" already exist ? */ + if (rc == -EEXIST) + GOTO(try_proc, rc = 0); + GOTO(no_osc, rc); + } + /* create "osc" entry for compatibility purposes */ dname.name = "osc"; dname.len = strlen(dname.name); @@ -1934,26 +1942,11 @@ static int __init osp_init(void) rc = symlink ? PTR_ERR(symlink) : -ENOMEM; GOTO(no_osc, rc); } - sym.typ_debugfs_entry = symlink; + sym->typ_debugfs_entry = symlink; } else { dput(symlink); } - kobj = kset_find_obj(lustre_kset, dname.name); - if (kobj) { - kobject_put(kobj); - goto try_proc; - } - - kobj = class_setup_tunables(dname.name); - if (IS_ERR(kobj)) { - rc = PTR_ERR(kobj); - if (sym.typ_debugfs_entry) - ldebugfs_remove(&sym.typ_debugfs_entry); - GOTO(no_osc, rc); - } - sym.typ_kobj = kobj; - try_proc: type = class_search_type(LUSTRE_OSC_NAME); if (type != NULL && type->typ_procroot != NULL) @@ -1979,8 +1972,10 @@ no_osc: */ static void __exit osp_exit(void) { - ldebugfs_remove(&sym.typ_debugfs_entry); - kobject_put(sym.typ_kobj); + if (!IS_ERR_OR_NULL(sym)) { + ldebugfs_remove(&sym->typ_debugfs_entry); + kobject_put(&sym->typ_kobj); + } class_unregister_type(LUSTRE_LWP_NAME); class_unregister_type(LUSTRE_OSP_NAME); lu_kmem_fini(osp_caches); -- 1.8.3.1