From 6158f83b1b17ee75e42f263e263a5d39ff8ca96f Mon Sep 17 00:00:00 2001 From: Fan Yong Date: Thu, 4 Sep 2014 00:29:41 +0800 Subject: [PATCH] LU-4604 obdclass: handle ldt_device_nr/ldt_linkage properly It is partly back-ported the patch from master: "LU-4604 lfsck: LFSCK async updates RPC flow control" But this only contains the part related with lu_device fixes, not the lfsck async RPC changes. There was no protection when inc/dec lu_device_type::ldt_device_nr, which may caused the ldt_device_nr to be wrong and trigger assert. This patch redefine lu_device_type::ldt_device_nr as atomic type. There was no protection when add/del lu_device_type::ldt_linkage into/from the global lu_device_types list, which may caused bad address accessing. This patch uses the existing obd_types_lock to protect related operations. We do NOT need lu_types_stop() any longer. Such function scans the global lu_device_types list, and for each type item on it which has zerod lu_device_type::ldt_device_nr, call its stop() method. In fact, the lu_device_type::ldt_device_nr only will be zero when the last lu_device_fini() is called, and at that time, inside the lu_device_fini(), its stop() method will be called. So it is unnecessary to call the stop() again via lu_types_stop(). Lustre-commit: caef708d4040fd13499a11a42507ba56f9454298 Lustre-change: http://review.whamcloud.com/8694 Signed-off-by: Fan Yong Change-Id: Ibea9e8f51191de76dbd2e199e01b1d2adff35c75 Reviewed-on: http://review.whamcloud.com/12531 Tested-by: Jenkins Tested-by: Maloo Reviewed-by: Mike Pershin Reviewed-by: Andreas Dilger --- lustre/include/lu_object.h | 3 +-- lustre/liblustre/llite_cl.c | 9 ++------- lustre/llite/vvp_dev.c | 9 ++------- lustre/obdclass/lu_object.c | 47 +++++++++++++++++++++++---------------------- lustre/obdclass/obd_mount.c | 3 +-- 5 files changed, 30 insertions(+), 41 deletions(-) diff --git a/lustre/include/lu_object.h b/lustre/include/lu_object.h index 991d101..1f26790 100644 --- a/lustre/include/lu_object.h +++ b/lustre/include/lu_object.h @@ -333,7 +333,7 @@ struct lu_device_type { /** * Number of existing device type instances. */ - unsigned ldt_device_nr; + atomic_t ldt_device_nr; /** * Linkage into a global list of all device types. * @@ -681,7 +681,6 @@ void lu_dev_del_linkage(struct lu_site *s, struct lu_device *d); int lu_device_type_init(struct lu_device_type *ldt); void lu_device_type_fini(struct lu_device_type *ldt); -void lu_types_stop(void); /** @} ctors */ diff --git a/lustre/liblustre/llite_cl.c b/lustre/liblustre/llite_cl.c index 630be49..2769a0a 100644 --- a/lustre/liblustre/llite_cl.c +++ b/lustre/liblustre/llite_cl.c @@ -786,11 +786,6 @@ int cl_sb_fini(struct llu_sb_info *sbi) sbi->ll_site = NULL; } cl_env_put(env, &refcheck); - /* - * If mount failed (sbi->ll_cl == NULL), and this there are no other - * mounts, stop device types manually (this usually happens - * automatically when last device is destroyed). - */ - lu_types_stop(); - RETURN(0); + + RETURN(0); } diff --git a/lustre/llite/vvp_dev.c b/lustre/llite/vvp_dev.c index d70a40b..9562f4e 100644 --- a/lustre/llite/vvp_dev.c +++ b/lustre/llite/vvp_dev.c @@ -244,13 +244,8 @@ int cl_sb_fini(struct super_block *sb) CERROR("Cannot cleanup cl-stack due to memory shortage.\n"); result = PTR_ERR(env); } - /* - * If mount failed (sbi->ll_cl == NULL), and this there are no other - * mounts, stop device types manually (this usually happens - * automatically when last device is destroyed). - */ - lu_types_stop(); - RETURN(result); + + RETURN(result); } /**************************************************************************** diff --git a/lustre/obdclass/lu_object.c b/lustre/obdclass/lu_object.c index 0b8df84..ed44efa 100644 --- a/lustre/obdclass/lu_object.c +++ b/lustre/obdclass/lu_object.c @@ -92,6 +92,8 @@ CFS_MODULE_PARM(lu_cache_nr, "l", long, 0644, static void lu_object_free(const struct lu_env *env, struct lu_object *o); +extern spinlock_t obd_types_lock; + /** * Decrease reference counter on object. If last reference is freed, return * object to the cache, unless lu_object_is_dying(o) holds. In the latter @@ -862,34 +864,30 @@ int lu_device_type_init(struct lu_device_type *ldt) { int result = 0; + atomic_set(&ldt->ldt_device_nr, 0); CFS_INIT_LIST_HEAD(&ldt->ldt_linkage); if (ldt->ldt_ops->ldto_init) result = ldt->ldt_ops->ldto_init(ldt); - if (result == 0) + + if (result == 0) { + spin_lock(&obd_types_lock); cfs_list_add(&ldt->ldt_linkage, &lu_device_types); + spin_unlock(&obd_types_lock); + } return result; } EXPORT_SYMBOL(lu_device_type_init); void lu_device_type_fini(struct lu_device_type *ldt) { + spin_lock(&obd_types_lock); cfs_list_del_init(&ldt->ldt_linkage); + spin_unlock(&obd_types_lock); if (ldt->ldt_ops->ldto_fini) ldt->ldt_ops->ldto_fini(ldt); } EXPORT_SYMBOL(lu_device_type_fini); -void lu_types_stop(void) -{ - struct lu_device_type *ldt; - - cfs_list_for_each_entry(ldt, &lu_device_types, ldt_linkage) { - if (ldt->ldt_device_nr == 0 && ldt->ldt_ops->ldto_stop) - ldt->ldt_ops->ldto_stop(ldt); - } -} -EXPORT_SYMBOL(lu_types_stop); - /** * Global list of all sites on this node */ @@ -1224,14 +1222,15 @@ EXPORT_SYMBOL(lu_device_put); */ int lu_device_init(struct lu_device *d, struct lu_device_type *t) { - if (t->ldt_device_nr++ == 0 && t->ldt_ops->ldto_start != NULL) - t->ldt_ops->ldto_start(t); - memset(d, 0, sizeof *d); - cfs_atomic_set(&d->ld_ref, 0); - d->ld_type = t; - lu_ref_init(&d->ld_reference); - CFS_INIT_LIST_HEAD(&d->ld_linkage); - return 0; + if (atomic_inc_return(&t->ldt_device_nr) == 1 && + t->ldt_ops->ldto_start != NULL) + t->ldt_ops->ldto_start(t); + + memset(d, 0, sizeof *d); + d->ld_type = t; + lu_ref_init(&d->ld_reference); + CFS_INIT_LIST_HEAD(&d->ld_linkage); + return 0; } EXPORT_SYMBOL(lu_device_init); @@ -1251,9 +1250,11 @@ void lu_device_fini(struct lu_device *d) lu_ref_fini(&d->ld_reference); LASSERTF(cfs_atomic_read(&d->ld_ref) == 0, "Refcount is %u\n", cfs_atomic_read(&d->ld_ref)); - LASSERT(t->ldt_device_nr > 0); - if (--t->ldt_device_nr == 0 && t->ldt_ops->ldto_stop != NULL) - t->ldt_ops->ldto_stop(t); + LASSERT(atomic_read(&t->ldt_device_nr) > 0); + + if (atomic_dec_and_test(&t->ldt_device_nr) && + t->ldt_ops->ldto_stop != NULL) + t->ldt_ops->ldto_stop(t); } EXPORT_SYMBOL(lu_device_fini); diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index 4dba8c2..28614fa 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -819,8 +819,7 @@ int lustre_common_put_super(struct super_block *sb) } /* Drop a ref to the mounted disk */ lustre_put_lsi(sb); - lu_types_stop(); - RETURN(rc); + RETURN(rc); } EXPORT_SYMBOL(lustre_common_put_super); -- 1.8.3.1