From e893964bace5097631c032f47b00b34129336517 Mon Sep 17 00:00:00 2001 From: yury <yury> Date: Thu, 23 Oct 2008 09:42:20 +0000 Subject: [PATCH] b=17447 r=adilger,deen - missed lustre_put_lsi() in couple of places; - do not call deregister_mount() in mount error path, this makes it impossible for MDT to do put_mount() and thus, its lsi left not released; - fixes error handling with llog_setup/llog_cleanup in couple of places; - fixes error handling after hash_init errors in class_setup(); - cleanups. --- lustre/lov/lov_log.c | 15 ++++++++++-- lustre/mdc/mdc_request.c | 27 +++++++++++++++++---- lustre/mds/handler.c | 14 +++++++---- lustre/mds/mds_log.c | 17 ++++++++++--- lustre/mgc/mgc_request.c | 4 ++++ lustre/mgs/mgs_handler.c | 7 +++++- lustre/obdclass/class_hash.c | 5 ++-- lustre/obdclass/llog_obd.c | 15 ++++++++++-- lustre/obdclass/obd_config.c | 27 ++++++++++++++------- lustre/obdclass/obd_mount.c | 46 +++++++++++++++++++----------------- lustre/osc/osc_request.c | 7 +++++- 11 files changed, 132 insertions(+), 52 deletions(-) diff --git a/lustre/lov/lov_log.c b/lustre/lov/lov_log.c index a5644104cd..eff8e777ae 100644 --- a/lustre/lov/lov_log.c +++ b/lustre/lov/lov_log.c @@ -212,7 +212,7 @@ int lov_llog_init(struct obd_device *obd, struct obd_device *tgt, rc = llog_setup(obd, LLOG_SIZE_REPL_CTXT, tgt, 0, NULL, &lov_size_repl_logops); if (rc) - RETURN(rc); + GOTO(err_cleanup, rc); lov_getref(obd); for (i = 0; i < lov->desc.ld_tgt_count ; i++) { @@ -233,7 +233,18 @@ int lov_llog_init(struct obd_device *obd, struct obd_device *tgt, } } lov_putref(obd); - RETURN(err); + GOTO(err_cleanup, err); +err_cleanup: + if (err) { + struct llog_ctxt *ctxt = + llog_get_context(obd, LLOG_SIZE_REPL_CTXT); + if (ctxt) + llog_cleanup(ctxt); + ctxt = llog_get_context(obd, LLOG_MDS_OST_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); + } + return err; } int lov_llog_finish(struct obd_device *obd, int count) diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c index d7f2da51b2..c786ff7980 100644 --- a/lustre/mdc/mdc_request.c +++ b/lustre/mdc/mdc_request.c @@ -1362,21 +1362,38 @@ static int mdc_llog_init(struct obd_device *obd, struct obd_device *tgt, ctxt = llog_get_context(obd, LLOG_LOVEA_REPL_CTXT); llog_initiator_connect(ctxt); llog_ctxt_put(ctxt); + } else { + GOTO(err_cleanup, rc); } RETURN(rc); +err_cleanup: + ctxt = llog_get_context(obd, LLOG_CONFIG_REPL_CTXT); + if (ctxt) + llog_cleanup(ctxt); + ctxt = llog_get_context(obd, LLOG_LOVEA_REPL_CTXT); + if (ctxt) + llog_cleanup(ctxt); + return rc; } static int mdc_llog_finish(struct obd_device *obd, int count) { - int rc; + struct llog_ctxt *ctxt; + int rc = 0; ENTRY; - rc = llog_cleanup(llog_get_context(obd, LLOG_LOVEA_REPL_CTXT)); - if (rc) { - CERROR("can not cleanup LLOG_CONFIG_REPL_CTXT rc %d\n", rc); + ctxt = llog_get_context(obd, LLOG_LOVEA_REPL_CTXT); + if (ctxt) { + rc = llog_cleanup(ctxt); + if (rc) { + CERROR("Can not cleanup LLOG_CONFIG_REPL_CTXT " + "rc %d\n", rc); + } } - rc = llog_cleanup(llog_get_context(obd, LLOG_CONFIG_REPL_CTXT)); + ctxt = llog_get_context(obd, LLOG_CONFIG_REPL_CTXT); + if (ctxt) + rc = llog_cleanup(ctxt); RETURN(rc); } diff --git a/lustre/mds/handler.c b/lustre/mds/handler.c index 8451ba0292..1a6ba925dc 100644 --- a/lustre/mds/handler.c +++ b/lustre/mds/handler.c @@ -2220,6 +2220,7 @@ static int mds_lov_clean(struct obd_device *obd) static int mds_postsetup(struct obd_device *obd) { struct mds_obd *mds = &obd->u.mds; + struct llog_ctxt *ctxt; int rc = 0; ENTRY; @@ -2231,7 +2232,7 @@ static int mds_postsetup(struct obd_device *obd) rc = llog_setup(obd, LLOG_LOVEA_ORIG_CTXT, obd, 0, NULL, &llog_lvfs_ops); if (rc) - RETURN(rc); + GOTO(err_llog, rc); if (mds->mds_profile) { struct lustre_profile *lprof; @@ -2254,9 +2255,14 @@ static int mds_postsetup(struct obd_device *obd) err_cleanup: mds_lov_clean(obd); - llog_cleanup(llog_get_context(obd, LLOG_CONFIG_ORIG_CTXT)); - llog_cleanup(llog_get_context(obd, LLOG_LOVEA_ORIG_CTXT)); - RETURN(rc); + ctxt = llog_get_context(obd, LLOG_LOVEA_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); +err_llog: + ctxt = llog_get_context(obd, LLOG_CONFIG_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); + return rc; } int mds_postrecov(struct obd_device *obd) diff --git a/lustre/mds/mds_log.c b/lustre/mds/mds_log.c index 21290e5d92..ca7fca0340 100644 --- a/lustre/mds/mds_log.c +++ b/lustre/mds/mds_log.c @@ -200,6 +200,7 @@ int mds_llog_init(struct obd_device *obd, struct obd_device *tgt, int count, struct llog_catid *logid, struct obd_uuid *uuid) { struct obd_device *lov_obd = obd->u.mds.mds_osc_obd; + struct llog_ctxt *ctxt; int rc; ENTRY; @@ -211,13 +212,23 @@ int mds_llog_init(struct obd_device *obd, struct obd_device *tgt, rc = llog_setup(obd, LLOG_SIZE_REPL_CTXT, tgt, 0, NULL, &mds_size_repl_logops); if (rc) - RETURN(rc); + GOTO(err_llog, rc); rc = obd_llog_init(lov_obd, tgt, count, logid, uuid); - if (rc) + if (rc) { CERROR("lov_llog_init err %d\n", rc); - + GOTO(err_cleanup, rc); + } RETURN(rc); +err_cleanup: + ctxt = llog_get_context(obd, LLOG_SIZE_REPL_CTXT); + if (ctxt) + llog_cleanup(ctxt); +err_llog: + ctxt = llog_get_context(obd, LLOG_MDS_OST_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); + return rc; } int mds_llog_finish(struct obd_device *obd, int count) diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c index 63bdd9ae67..28c3bd48e4 100644 --- a/lustre/mgc/mgc_request.c +++ b/lustre/mgc/mgc_request.c @@ -938,6 +938,10 @@ static int mgc_llog_init(struct obd_device *obd, struct obd_device *tgt, ctxt = llog_get_context(obd, LLOG_CONFIG_REPL_CTXT); llog_initiator_connect(ctxt); llog_ctxt_put(ctxt); + } else { + ctxt = llog_get_context(obd, LLOG_CONFIG_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); } RETURN(rc); diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c index 0f0cb58296..93d6a2e503 100644 --- a/lustre/mgs/mgs_handler.c +++ b/lustre/mgs/mgs_handler.c @@ -163,6 +163,7 @@ static int mgs_setup(struct obd_device *obd, obd_count len, void *buf) struct mgs_obd *mgs = &obd->u.mgs; struct lustre_mount_info *lmi; struct lustre_sb_info *lsi; + struct llog_ctxt *ctxt; struct vfsmount *mnt; int rc = 0; ENTRY; @@ -223,7 +224,7 @@ static int mgs_setup(struct obd_device *obd, obd_count len, void *buf) if (!mgs->mgs_service) { CERROR("failed to start service\n"); - GOTO(err_fs, rc = -ENOMEM); + GOTO(err_llog, rc = -ENOMEM); } rc = ptlrpc_start_threads(obd, mgs->mgs_service); @@ -244,6 +245,10 @@ static int mgs_setup(struct obd_device *obd, obd_count len, void *buf) err_thread: ptlrpc_unregister_service(mgs->mgs_service); +err_llog: + ctxt = llog_get_context(obd, LLOG_CONFIG_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); err_fs: /* No extra cleanup needed for llog_init_commit_thread() */ mgs_fs_cleanup(obd); diff --git a/lustre/obdclass/class_hash.c b/lustre/obdclass/class_hash.c index 919fa4ac1e..4138cc097a 100644 --- a/lustre/obdclass/class_hash.c +++ b/lustre/obdclass/class_hash.c @@ -127,9 +127,8 @@ lustre_hash_exit(lustre_hash_t *lh) struct hlist_node *pos; int i; ENTRY; - - if (!lh) - return; + + LASSERT(lh != NULL); write_lock(&lh->lh_rwlock); diff --git a/lustre/obdclass/llog_obd.c b/lustre/obdclass/llog_obd.c index bb848bdbbd..b9b41fcd3b 100644 --- a/lustre/obdclass/llog_obd.c +++ b/lustre/obdclass/llog_obd.c @@ -90,7 +90,11 @@ int __llog_ctxt_put(struct llog_ctxt *ctxt) obd->obd_llog_ctxt[ctxt->loc_idx] = NULL; spin_unlock(&obd->obd_dev_lock); - LASSERT(obd->obd_stopping == 1 || obd->obd_set_up == 0); + LASSERTF(obd->obd_starting == 1 || + obd->obd_stopping == 1 || obd->obd_set_up == 0, + "wrong obd state: %d/%d/%d\n", !!obd->obd_starting, + !!obd->obd_stopping, !!obd->obd_set_up); + /* cleanup the llog ctxt here */ if (CTXTP(ctxt, cleanup)) rc = CTXTP(ctxt, cleanup)(ctxt); @@ -118,7 +122,14 @@ int llog_cleanup(struct llog_ctxt *ctxt) /* sync with other llog ctxt user thread */ spin_lock(&obd->obd_dev_lock); - LASSERT(obd->obd_stopping == 1 || obd->obd_set_up == 0); + + /* obd->obd_starting is needed for the case of cleanup + * in error case while obd is starting up. */ + LASSERTF(obd->obd_starting == 1 || + obd->obd_stopping == 1 || obd->obd_set_up == 0, + "wrong obd state: %d/%d/%d\n", !!obd->obd_starting, + !!obd->obd_stopping, !!obd->obd_set_up); + spin_unlock(&obd->obd_dev_lock); idx = ctxt->loc_idx; diff --git a/lustre/obdclass/obd_config.c b/lustre/obdclass/obd_config.c index 8745560d0f..e1cedeb477 100644 --- a/lustre/obdclass/obd_config.c +++ b/lustre/obdclass/obd_config.c @@ -307,23 +307,24 @@ int class_setup(struct obd_device *obd, struct lustre_cfg *lcfg) obd->obd_uuid_hash = lustre_hash_init("UUID_HASH", 128, 128, &uuid_hash_ops, 0); if (!obd->obd_uuid_hash) - GOTO(err_hash, -ENOMEM); + GOTO(err_hash, err = -ENOMEM); /* create a nid-export lustre hash */ obd->obd_nid_hash = lustre_hash_init("NID_HASH", 128, 128, &nid_hash_ops, 0); if (!obd->obd_nid_hash) - GOTO(err_hash, -ENOMEM); + GOTO(err_hash, err = -ENOMEM); /* create a nid-stats lustre hash */ obd->obd_nid_stats_hash = lustre_hash_init("NID_STATS", 128, 128, &nid_stat_hash_ops, 0); if (!obd->obd_nid_stats_hash) - GOTO(err_hash, -ENOMEM); + GOTO(err_hash, err = -ENOMEM); exp = class_new_export(obd, &obd->obd_uuid); if (IS_ERR(exp)) - RETURN(PTR_ERR(exp)); + GOTO(err_hash, err = PTR_ERR(exp)); + obd->obd_self_export = exp; list_del_init(&exp->exp_obd_chain_timed); class_export_put(exp); @@ -342,17 +343,25 @@ int class_setup(struct obd_device *obd, struct lustre_cfg *lcfg) obd->obd_name, obd->obd_uuid.uuid); RETURN(0); - err_exp: class_unlink_export(obd->obd_self_export); obd->obd_self_export = NULL; err_hash: - lustre_hash_exit(obd->obd_uuid_hash); - lustre_hash_exit(obd->obd_nid_hash); - lustre_hash_exit(obd->obd_nid_stats_hash); + if (obd->obd_uuid_hash) { + lustre_hash_exit(obd->obd_uuid_hash); + obd->obd_uuid_hash = NULL; + } + if (obd->obd_nid_hash) { + lustre_hash_exit(obd->obd_nid_hash); + obd->obd_nid_hash = NULL; + } + if (obd->obd_nid_stats_hash) { + lustre_hash_exit(obd->obd_nid_stats_hash); + obd->obd_nid_stats_hash = NULL; + } obd->obd_starting = 0; CERROR("setup %s failed (%d)\n", obd->obd_name, err); - RETURN(err); + return err; } int class_detach(struct obd_device *obd, struct lustre_cfg *lcfg) diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index df95ebab90..635da45345 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -167,7 +167,7 @@ struct lustre_mount_info *server_get_mount(char *name) lsi = s2lsi(lmi->lmi_sb); mntget(lmi->lmi_mnt); atomic_inc(&lsi->lsi_mounts); - + CDEBUG(D_MOUNT, "get_mnt %p from %s, refs=%d, vfscount=%d\n", lmi->lmi_mnt, name, atomic_read(&lsi->lsi_mounts), atomic_read(&lmi->lmi_mnt->mnt_count)); @@ -1111,7 +1111,6 @@ static int server_start_targets(struct super_block *sb, struct vfsmount *mnt) if (rc) { CERROR("failed to start server %s: %d\n", lsi->lsi_ldd->ldd_svname, rc); - server_deregister_mount(lsi->lsi_ldd->ldd_svname); GOTO(out_mgc, rc); } @@ -1171,10 +1170,8 @@ static int lustre_free_lsi(struct super_block *sb) struct lustre_sb_info *lsi = s2lsi(sb); ENTRY; - if (!lsi) - RETURN(0); - - CDEBUG(D_MOUNT, "Freeing lsi\n"); + LASSERT(lsi != NULL); + CDEBUG(D_MOUNT, "Freeing lsi %p\n", lsi); /* someone didn't call server_put_mount. */ LASSERT(atomic_read(&lsi->lsi_mounts) == 0); @@ -1214,10 +1211,9 @@ static int lustre_put_lsi(struct super_block *sb) struct lustre_sb_info *lsi = s2lsi(sb); ENTRY; - LASSERT(lsi); + LASSERT(lsi != NULL); CDEBUG(D_MOUNT, "put %p %d\n", sb, atomic_read(&lsi->lsi_mounts)); - if (atomic_dec_and_test(&lsi->lsi_mounts)) { lustre_free_lsi(sb); RETURN(1); @@ -1588,9 +1584,9 @@ static int server_fill_super(struct super_block *sb) if (IS_ERR(mnt)) { rc = PTR_ERR(mnt); CERROR("Unable to mount device %s: %d\n", - lsi->lsi_lmd->lmd_dev, rc); + lsi->lsi_lmd->lmd_dev, rc); lustre_put_lsi(sb); - GOTO(out, rc); + RETURN(rc); } lsi->lsi_srv_mnt = mnt; @@ -1604,12 +1600,12 @@ static int server_fill_super(struct super_block *sb) "running. Double-mount may have compromised " "the disk journal.\n", lsi->lsi_ldd->ldd_svname); - unlock_mntput(mnt); lustre_put_lsi(sb); - GOTO(out, rc = -EALREADY); + unlock_mntput(mnt); + RETURN(-EALREADY); } - /* start MGS before MGC */ + /* Start MGS before MGC */ if (IS_MGS(lsi->lsi_ldd) && !(lsi->lsi_lmd->lmd_flags & LMD_FLG_NOMGS)) { rc = server_start_mgs(sb); if (rc) @@ -1647,11 +1643,12 @@ static int server_fill_super(struct super_block *sb) lsi->lsi_ldd->ldd_svname, lsi->lsi_lmd->lmd_dev); RETURN(0); - out_mnt: + /* We jump here in case of failure while starting targets or MGS. + * In this case we can't just put @mnt and have to do real cleanup + * with stoping targets, etc. */ server_put_super(sb); -out: - RETURN(rc); + return rc; } /* Get the index from the obd name. @@ -1948,7 +1945,7 @@ int lustre_fill_super(struct super_block *sb, void *data, int silent) /* Figure out the lmd from the mount options */ if (lmd_parse((char *)data, lmd)) { lustre_put_lsi(sb); - RETURN(-EINVAL); + GOTO(out, rc = -EINVAL); } if (lmd_is_client(lmd)) { @@ -1957,18 +1954,19 @@ int lustre_fill_super(struct super_block *sb, void *data, int silent) LCONSOLE_ERROR_MSG(0x165, "Nothing registered for " "client mount! Is the 'lustre' " "module loaded?\n"); + lustre_put_lsi(sb); rc = -ENODEV; } else { rc = lustre_start_mgc(sb); if (rc) { lustre_stop_mgc(sb); - goto out; + lustre_put_lsi(sb); + GOTO(out, rc); } /* Connect and start */ /* (should always be ll_fill_super) */ rc = (*client_fill_super)(sb); /* c_f_s will call lustre_common_put_super on failure */ - } } else { CDEBUG(D_MOUNT, "Mounting server from %s\n", lmd->lmd_dev); @@ -1980,14 +1978,18 @@ int lustre_fill_super(struct super_block *sb, void *data, int silent) /* s_f_s will call server_put_super on failure */ } + /* If error happens in fill_super() call, @lsi will be killed there. + * This is why we do not put it here. */ + GOTO(out, rc); out: - if (rc){ + if (rc) { CERROR("Unable to mount %s (%d)\n", s2lsi(sb) ? lmd->lmd_dev : "", rc); } else { - CDEBUG(D_SUPER, "mount %s complete\n", lmd->lmd_dev); + CDEBUG(D_SUPER, "Mount %s complete\n", + lmd->lmd_dev); } - RETURN(rc); + return rc; } diff --git a/lustre/osc/osc_request.c b/lustre/osc/osc_request.c index f21351cb43..9c6665495a 100644 --- a/lustre/osc/osc_request.c +++ b/lustre/osc/osc_request.c @@ -3662,8 +3662,13 @@ static int osc_llog_init(struct obd_device *obd, struct obd_device *tgt, rc = llog_setup(obd, LLOG_SIZE_REPL_CTXT, tgt, count, NULL, &osc_size_repl_logops); - if (rc) + if (rc) { + struct llog_ctxt *ctxt = + llog_get_context(obd, LLOG_MDS_OST_ORIG_CTXT); + if (ctxt) + llog_cleanup(ctxt); CERROR("failed LLOG_SIZE_REPL_CTXT\n"); + } out: if (rc) { CERROR("osc '%s' tgt '%s' cnt %d catid %p rc=%d\n", -- GitLab