From c228613e18d4496d026d56040e394fe90273de2f Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Tue, 17 Sep 2019 21:17:20 -0400 Subject: [PATCH] LU-8066 obd_type: discard obd_types linked list. As all obd_types are kobjects in the lustre_kset kset, they are linked together in that kset and don't need any extra linkage. There are non-obd_type objects in lustre_kset, added by class_setup_tunables(). These have a different ->ktype, so we are careful to only return objects with the correct ->ktype. As kset_find_obj() returns a counted reference, we need to put that reference when done. On the server side it is possible to have an obd_type partially initialized by one subsystem and latter fully initialized by another subsystem. We use typ_sym_filter to notify us if the obd_type is only partially setup. If it only paritially setup then we let the original subsystem that created the obd_type to clean it up. If the obd_type was latter completely setup then we let the latter subsystem do the cleanup for us. Linux-commit: 881bc9b58ef5e8c9be297b121187ea6c26572cf1 Change-Id: I4316644f7fb12e358b799af64deb57836e796066 Signed-off-by: NeilBrown Reviewed-on: https://review.whamcloud.com/34718 Reviewed-by: Wang Shilong Tested-by: jenkins Reviewed-by: Andreas Dilger Tested-by: Maloo Reviewed-by: Petros Koutoupis Reviewed-by: Neil Brown Reviewed-by: Alexandr Boyko Reviewed-by: Oleg Drokin --- lustre/include/obd.h | 1 - lustre/include/obd_class.h | 2 + lustre/lod/lod_dev.c | 15 +++++-- lustre/mdd/mdd_lproc.c | 3 ++ lustre/obdclass/genops.c | 83 ++++++++++++++++---------------------- lustre/obdclass/obd_mount_server.c | 3 ++ lustre/osd-ldiskfs/osd_internal.h | 2 +- lustre/osd-ldiskfs/osd_lproc.c | 3 ++ lustre/osd-zfs/osd_lproc.c | 3 ++ lustre/osp/osp_dev.c | 17 ++++++-- lustre/quota/qmt_dev.c | 3 ++ 11 files changed, 77 insertions(+), 58 deletions(-) diff --git a/lustre/include/obd.h b/lustre/include/obd.h index 9cdf306..c43cafb 100644 --- a/lustre/include/obd.h +++ b/lustre/include/obd.h @@ -99,7 +99,6 @@ struct obd_info { }; struct obd_type { - struct list_head typ_chain; struct obd_ops *typ_dt_ops; struct md_ops *typ_md_ops; struct proc_dir_entry *typ_procroot; diff --git a/lustre/include/obd_class.h b/lustre/include/obd_class.h index 29728e6..11ddb0e 100644 --- a/lustre/include/obd_class.h +++ b/lustre/include/obd_class.h @@ -359,8 +359,10 @@ void class_import_put(struct obd_import *); struct obd_import *class_new_import(struct obd_device *obd); void class_destroy_import(struct obd_import *exp); +#ifdef HAVE_SERVER_SUPPORT struct obd_type *class_search_type(const char *name); struct obd_type *class_get_type(const char *name); +#endif void class_put_type(struct obd_type *type); int class_connect(struct lustre_handle *conn, struct obd_device *obd, struct obd_uuid *cluuid); diff --git a/lustre/lod/lod_dev.c b/lustre/lod/lod_dev.c index a452e5d..99ccbb0 100644 --- a/lustre/lod/lod_dev.c +++ b/lustre/lod/lod_dev.c @@ -2166,10 +2166,9 @@ static struct obd_ops lod_obd_device_ops = { .o_pool_del = lod_pool_del, }; -static struct obd_type *sym; - static int __init lod_init(void) { + struct obd_type *sym; int rc; rc = lu_kmem_init(lod_caches); @@ -2197,8 +2196,18 @@ static int __init lod_init(void) static void __exit lod_exit(void) { - if (!IS_ERR_OR_NULL(sym)) + struct obd_type *sym = class_search_type(LUSTRE_LOV_NAME); + + /* if this was never fully initialized by the lov layer + * then we are responsible for freeing this obd_type + */ + if (sym) { + /* final put if we manage this obd type */ + if (sym->typ_sym_filter) + kobject_put(&sym->typ_kobj); + /* put reference taken by class_search_type */ 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 f18a6ff..829afc7 100644 --- a/lustre/mdd/mdd_lproc.c +++ b/lustre/mdd/mdd_lproc.c @@ -702,6 +702,9 @@ int mdd_procfs_init(struct mdd_device *mdd, const char *name) LASSERT(type != NULL); LASSERT(obd != NULL); + /* put reference taken by class_search_type */ + kobject_put(&type->typ_kobj); + mdd->mdd_ktype.default_attrs = mdd_attrs; mdd->mdd_ktype.release = mdd_sysfs_release; mdd->mdd_ktype.sysfs_ops = &lustre_sysfs_ops; diff --git a/lustre/obdclass/genops.c b/lustre/obdclass/genops.c index df1a31d..e622041 100644 --- a/lustre/obdclass/genops.c +++ b/lustre/obdclass/genops.c @@ -46,13 +46,11 @@ #include #include -static DEFINE_SPINLOCK(obd_types_lock); -static LIST_HEAD(obd_types); DEFINE_RWLOCK(obd_dev_lock); static struct obd_device *obd_devs[MAX_OBD_DEVICES]; static struct kmem_cache *obd_device_cachep; - +static struct kobj_type class_ktype; static struct workqueue_struct *zombie_wq; static void obd_zombie_export_add(struct obd_export *exp); @@ -98,30 +96,26 @@ static void obd_device_free(struct obd_device *obd) struct obd_type *class_search_type(const char *name) { - struct list_head *tmp; - struct obd_type *type; + struct kobject *kobj = kset_find_obj(lustre_kset, name); - spin_lock(&obd_types_lock); - list_for_each(tmp, &obd_types) { - type = list_entry(tmp, struct obd_type, typ_chain); - if (strcmp(type->typ_name, name) == 0) { - spin_unlock(&obd_types_lock); - return type; - } - } - spin_unlock(&obd_types_lock); + if (kobj && kobj->ktype == &class_ktype) + return container_of(kobj, struct obd_type, typ_kobj); + + kobject_put(kobj); return NULL; } EXPORT_SYMBOL(class_search_type); struct obd_type *class_get_type(const char *name) { - struct obd_type *type = class_search_type(name); + struct obd_type *type; + type = class_search_type(name); #ifdef HAVE_MODULE_LOADING_SUPPORT if (!type) { const char *modname = name; +#ifdef HAVE_SERVER_SUPPORT if (strcmp(modname, "obdfilter") == 0) modname = "ofd"; @@ -130,6 +124,7 @@ struct obd_type *class_get_type(const char *name) if (!strncmp(modname, LUSTRE_MDS_NAME, strlen(LUSTRE_MDS_NAME))) modname = LUSTRE_MDT_NAME; +#endif /* HAVE_SERVER_SUPPORT */ if (!request_module("%s", modname)) { CDEBUG(D_INFO, "Loaded module '%s'\n", modname); @@ -145,6 +140,11 @@ struct obd_type *class_get_type(const char *name) type->typ_refcnt++; try_module_get(type->typ_dt_ops->o_owner); spin_unlock(&type->obd_type_lock); + /* class_search_type() returned a counted reference, + * but we don't need that count any more as + * we have one through typ_refcnt. + */ + kobject_put(&type->typ_kobj); } return type; } @@ -162,20 +162,12 @@ static void class_sysfs_release(struct kobject *kobj) { struct obd_type *type = container_of(kobj, struct obd_type, typ_kobj); -#ifdef HAVE_SERVER_SUPPORT - if (type->typ_sym_filter) - type->typ_debugfs_entry = NULL; -#endif debugfs_remove_recursive(type->typ_debugfs_entry); type->typ_debugfs_entry = NULL; if (type->typ_lu) lu_device_type_fini(type->typ_lu); - spin_lock(&obd_types_lock); - list_del(&type->typ_chain); - spin_unlock(&obd_types_lock); - #ifdef CONFIG_PROC_FS if (type->typ_name && type->typ_procroot) remove_proc_subtree(type->typ_name, proc_lustre_root); @@ -198,12 +190,11 @@ struct obd_type *class_add_symlinks(const char *name, bool enable_proc) { struct dentry *symlink; struct obd_type *type; - struct kobject *kobj; int rc; - kobj = kset_find_obj(lustre_kset, name); - if (kobj) { - kobject_put(kobj); + type = class_search_type(name); + if (type) { + kobject_put(&type->typ_kobj); return ERR_PTR(-EEXIST); } @@ -211,8 +202,6 @@ struct obd_type *class_add_symlinks(const char *name, bool enable_proc) if (!type) return ERR_PTR(-ENOMEM); - INIT_LIST_HEAD(&type->typ_chain); - type->typ_kobj.kset = lustre_kset; rc = kobject_init_and_add(&type->typ_kobj, &class_ktype, &lustre_kset->kobj, "%s", name); @@ -256,20 +245,13 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, /* sanity check */ LASSERT(strnlen(name, CLASS_MAX_NAME) < CLASS_MAX_NAME); - if (class_search_type(name)) { + type = class_search_type(name); + if (type) { #ifdef HAVE_SERVER_SUPPORT - if (strcmp(name, LUSTRE_LOV_NAME) == 0 || - strcmp(name, LUSTRE_OSC_NAME) == 0) { - struct kobject *kobj; - - kobj = kset_find_obj(lustre_kset, name); - if (kobj) { - type = container_of(kobj, struct obd_type, - typ_kobj); - goto dir_exist; - } - } + if (type->typ_sym_filter) + goto dir_exist; #endif /* HAVE_SERVER_SUPPORT */ + kobject_put(&type->typ_kobj); CDEBUG(D_IOCTL, "Type %s already registered\n", name); RETURN(-EEXIST); } @@ -278,7 +260,6 @@ int class_register_type(struct obd_ops *dt_ops, struct md_ops *md_ops, if (type == NULL) RETURN(-ENOMEM); - INIT_LIST_HEAD(&type->typ_chain); type->typ_kobj.kset = lustre_kset; kobject_init(&type->typ_kobj, &class_ktype); #ifdef HAVE_SERVER_SUPPORT @@ -298,8 +279,11 @@ dir_exist: spin_lock_init(&type->obd_type_lock); #ifdef HAVE_SERVER_SUPPORT - if (type->typ_sym_filter) + if (type->typ_sym_filter) { + type->typ_sym_filter = false; + kobject_put(&type->typ_kobj); goto setup_ldt; + } #endif #ifdef CONFIG_PROC_FS if (enable_proc && !type->typ_procroot) { @@ -335,10 +319,6 @@ setup_ldt: GOTO(failed, rc); } - spin_lock(&obd_types_lock); - list_add(&type->typ_chain, &obd_types); - spin_unlock(&obd_types_lock); - RETURN(0); failed: @@ -351,6 +331,7 @@ EXPORT_SYMBOL(class_register_type); int class_unregister_type(const char *name) { struct obd_type *type = class_search_type(name); + int rc = 0; ENTRY; if (!type) { @@ -364,12 +345,16 @@ int class_unregister_type(const char *name) /* Remove ops, but leave the name for debugging */ OBD_FREE_PTR(type->typ_dt_ops); OBD_FREE_PTR(type->typ_md_ops); - RETURN(-EBUSY); + GOTO(out_put, rc = -EBUSY); } + /* Put the final ref */ + kobject_put(&type->typ_kobj); +out_put: + /* Put the ref returned by class_search_type() */ kobject_put(&type->typ_kobj); - RETURN(0); + RETURN(rc); } /* class_unregister_type */ EXPORT_SYMBOL(class_unregister_type); diff --git a/lustre/obdclass/obd_mount_server.c b/lustre/obdclass/obd_mount_server.c index 8ca8180..5b5cb20 100644 --- a/lustre/obdclass/obd_mount_server.c +++ b/lustre/obdclass/obd_mount_server.c @@ -1110,6 +1110,9 @@ static int server_stop_servers(int lsiflags) rc = class_manual_cleanup(obd); } + /* put reference taken by class_search_type */ + kobject_put(&type->typ_kobj); + mutex_unlock(&server_start_lock); RETURN(rc); diff --git a/lustre/osd-ldiskfs/osd_internal.h b/lustre/osd-ldiskfs/osd_internal.h index c29e53d..fe9c8f0 100644 --- a/lustre/osd-ldiskfs/osd_internal.h +++ b/lustre/osd-ldiskfs/osd_internal.h @@ -55,7 +55,7 @@ /* LUSTRE_OSD_NAME */ #include -/* class_register_type(), class_unregister_type(), class_get_type() */ +/* class_register_type(), class_unregister_type() */ #include #include #include diff --git a/lustre/osd-ldiskfs/osd_lproc.c b/lustre/osd-ldiskfs/osd_lproc.c index cba3a82..80dc25f 100644 --- a/lustre/osd-ldiskfs/osd_lproc.c +++ b/lustre/osd-ldiskfs/osd_lproc.c @@ -729,6 +729,9 @@ int osd_procfs_init(struct osd_device *osd, const char *name) LCONSOLE_INFO("osd-ldiskfs create tunables for %s\n", name); + /* put reference taken by class_search_type */ + kobject_put(&type->typ_kobj); + osd->od_dt_dev.dd_ktype.default_attrs = ldiskfs_attrs; rc = dt_tunables_init(&osd->od_dt_dev, type, name, lprocfs_osd_obd_vars); diff --git a/lustre/osd-zfs/osd_lproc.c b/lustre/osd-zfs/osd_lproc.c index a1becc1..1516e58 100644 --- a/lustre/osd-zfs/osd_lproc.c +++ b/lustre/osd-zfs/osd_lproc.c @@ -437,6 +437,9 @@ int osd_procfs_init(struct osd_device *osd, const char *name) LASSERT(type); LASSERT(name); + /* put reference taken by class_search_type */ + kobject_put(&type->typ_kobj); + osd->od_dt_dev.dd_ktype.default_attrs = zfs_attrs; rc = dt_tunables_init(&osd->od_dt_dev, type, name, lprocfs_osd_obd_vars); diff --git a/lustre/osp/osp_dev.c b/lustre/osp/osp_dev.c index dc3249b..eb768d4 100644 --- a/lustre/osp/osp_dev.c +++ b/lustre/osp/osp_dev.c @@ -1887,8 +1887,6 @@ static struct obd_ops osp_obd_device_ops = { .o_fid_alloc = osp_fid_alloc, }; -static struct obd_type *sym; - /** * Initialize OSP module. * @@ -1903,6 +1901,7 @@ static struct obd_type *sym; */ static int __init osp_init(void) { + struct obd_type *sym; int rc; rc = lu_kmem_init(osp_caches); @@ -1925,7 +1924,7 @@ static int __init osp_init(void) } /* create "osc" entry for compatibility purposes */ - sym = class_add_symlinks(LUSTRE_OSC_NAME, false); + sym = class_add_symlinks(LUSTRE_OSC_NAME, true); if (IS_ERR(sym)) { rc = PTR_ERR(sym); /* does real "osc" already exist ? */ @@ -1944,8 +1943,18 @@ static int __init osp_init(void) */ static void __exit osp_exit(void) { - if (!IS_ERR_OR_NULL(sym)) + struct obd_type *sym = class_search_type(LUSTRE_OSC_NAME); + + /* if this was never fully initialized by the osc layer + * then we are responsible for freeing this obd_type + */ + if (sym) { + /* final put if we manage this obd type */ + if (sym->typ_sym_filter) + kobject_put(&sym->typ_kobj); + /* put reference taken by class_search_type */ kobject_put(&sym->typ_kobj); + } class_unregister_type(LUSTRE_LWP_NAME); class_unregister_type(LUSTRE_OSP_NAME); diff --git a/lustre/quota/qmt_dev.c b/lustre/quota/qmt_dev.c index fe65065..19605fa 100644 --- a/lustre/quota/qmt_dev.c +++ b/lustre/quota/qmt_dev.c @@ -259,6 +259,9 @@ static int qmt_device_init0(const struct lu_env *env, struct qmt_device *qmt, type = class_search_type(LUSTRE_QMT_NAME); LASSERT(type != NULL); + /* put reference taken by class_search_type */ + kobject_put(&type->typ_kobj); + /* register proc directory associated with this qmt */ qmt->qmt_proc = lprocfs_register(qmt->qmt_svname, type->typ_procroot, NULL, NULL); -- 1.8.3.1