From 156a156b699bde7a7ad68ff429cad862d7a03196 Mon Sep 17 00:00:00 2001 From: nathan Date: Wed, 22 Feb 2006 00:04:26 +0000 Subject: [PATCH] Branch b1_4_mountconf b=8007 Hunt down some memory leaks: 1. There are problems with the llog_finish/init pair in mds_lov_add_ost. Firstly, they must be atomic as a pair. Secondly, the setup sets up more than the cleanup cleans up (see note in llog_setup), so we hackishly fix that. 2. Make sure we don't exit the umount until after we've dropped all our references to the mount. Class_cleanup isn't synchronous, and we can't exit the umount and risk our sb getting destroyed until we're really done. So added a polling check. Also, move the MDS/OSS cleanup check until after the poller, since I'm just looking at the class refcount. --- lustre/mds/mds_lov.c | 4 +++ lustre/mgs/mgs_handler.c | 12 +++---- lustre/mgs/mgs_llog.c | 1 + lustre/obdclass/llog_obd.c | 50 +++++++++++++++----------- lustre/obdclass/obd_mount.c | 85 +++++++++++++++++++++++++++++---------------- 5 files changed, 96 insertions(+), 56 deletions(-) diff --git a/lustre/mds/mds_lov.c b/lustre/mds/mds_lov.c index c3e7567..95f2e93 100644 --- a/lustre/mds/mds_lov.c +++ b/lustre/mds/mds_lov.c @@ -283,8 +283,12 @@ static int mds_lov_add_ost(struct obd_device *obd, struct obd_device *watched, CWARN("last object "LPU64" from OST %d\n", mds->mds_lov_objids[idx], idx); + + /* These two must be atomic */ + down(&mds->mds_orphan_recovery_sem); obd_llog_finish(obd, old_count); llog_cat_initialize(obd, mds->mds_lov_desc.ld_tgt_count); + up(&mds->mds_orphan_recovery_sem); RETURN(rc); } diff --git a/lustre/mgs/mgs_handler.c b/lustre/mgs/mgs_handler.c index 7bccab85..cd65026 100644 --- a/lustre/mgs/mgs_handler.c +++ b/lustre/mgs/mgs_handler.c @@ -570,13 +570,11 @@ int mgs_iocontrol(unsigned int cmd, struct obd_export *exp, int len, if (lcfg == NULL) RETURN(-ENOMEM); rc = copy_from_user(lcfg, data->ioc_pbuf1, data->ioc_plen1); - if (rc) { - OBD_FREE(lcfg, data->ioc_plen1); - RETURN(rc); - } + if (rc) + GOTO(out_free, rc); if (lcfg->lcfg_bufcount < 1) - RETURN(-EINVAL); + GOTO(out_free, rc = -EINVAL); /* Extract fsname */ /* FIXME COMPAT_146 this won't work with old names */ @@ -606,7 +604,9 @@ int mgs_iocontrol(unsigned int cmd, struct obd_export *exp, int len, } /* Revoke lock so everyone updates. Should be alright if - someone was reading while we were updating the logs. */ + someone was already reading while we were updating the logs, + so we don't really need to hold the lock while we're + writing (above). */ lockrc = mgs_get_cfg_lock(obd, fsname, &lockh); if (lockrc != ELDLM_OK) CERROR("lock error %d for fs %s\n", lockrc, fsname); diff --git a/lustre/mgs/mgs_llog.c b/lustre/mgs/mgs_llog.c index 78d3bc1..9fe3af1 100644 --- a/lustre/mgs/mgs_llog.c +++ b/lustre/mgs/mgs_llog.c @@ -1352,6 +1352,7 @@ int mgs_setparam(struct obd_device *obd, char *fsname, struct lustre_cfg *lcfg) lovname, lcfg); } name_destroy(lovname); + name_destroy(logname); up(&fsdb->fsdb_sem); } diff --git a/lustre/obdclass/llog_obd.c b/lustre/obdclass/llog_obd.c index 0e64454..40d548e 100644 --- a/lustre/obdclass/llog_obd.c +++ b/lustre/obdclass/llog_obd.c @@ -41,6 +41,26 @@ /* helper functions for calling the llog obd methods */ +int llog_cleanup(struct llog_ctxt *ctxt) +{ + int rc = 0; + ENTRY; + + if (!ctxt) { + CERROR("No ctxt\n"); + RETURN(-ENODEV); + } + + if (CTXTP(ctxt, cleanup)) + rc = CTXTP(ctxt, cleanup)(ctxt); + + ctxt->loc_obd->obd_llog_ctxt[ctxt->loc_idx] = NULL; + OBD_FREE(ctxt, sizeof(*ctxt)); + + RETURN(rc); +} +EXPORT_SYMBOL(llog_cleanup); + int llog_setup(struct obd_device *obd, int index, struct obd_device *disk_obd, int count, struct llog_logid *logid, struct llog_operations *op) { @@ -51,6 +71,16 @@ int llog_setup(struct obd_device *obd, int index, struct obd_device *disk_obd, if (index < 0 || index >= LLOG_MAX_CTXTS) RETURN(-EFAULT); + if (obd->obd_llog_ctxt[index]) { + /* During an mds_lov_add_ost, we try to tear down and resetup llogs. + But the mdt teardown does not flow down to the lov/osc's as the + setup does, because the lov/osc must clean up only when they are + done, not when the mdt is done. So instead, we just assume that + if the lov llogs are already set up then we must cleanup first. */ + CERROR("obd %s ctxt %d already set up\n", obd->obd_name, index); + llog_cleanup(obd->obd_llog_ctxt[index]); + } + OBD_ALLOC(ctxt, sizeof(*ctxt)); if (!ctxt) RETURN(-ENOMEM); @@ -69,26 +99,6 @@ int llog_setup(struct obd_device *obd, int index, struct obd_device *disk_obd, } EXPORT_SYMBOL(llog_setup); -int llog_cleanup(struct llog_ctxt *ctxt) -{ - int rc = 0; - ENTRY; - - if (!ctxt) { - CERROR("No ctxt\n"); - RETURN(-ENODEV); - } - - if (CTXTP(ctxt, cleanup)) - rc = CTXTP(ctxt, cleanup)(ctxt); - - ctxt->loc_obd->obd_llog_ctxt[ctxt->loc_idx] = NULL; - OBD_FREE(ctxt, sizeof(*ctxt)); - - RETURN(rc); -} -EXPORT_SYMBOL(llog_cleanup); - int llog_sync(struct llog_ctxt *ctxt, struct obd_export *exp) { int rc = 0; diff --git a/lustre/obdclass/obd_mount.c b/lustre/obdclass/obd_mount.c index 2fa8d31..57d1650 100644 --- a/lustre/obdclass/obd_mount.c +++ b/lustre/obdclass/obd_mount.c @@ -741,42 +741,36 @@ static int server_mgc_clear_fs(struct obd_device *mgc) } /* Stop MDS/OSS if nobody is using them */ -static int server_stop_servers(struct super_block *sb) +static int server_stop_servers(int lddflags, int lsiflags) { - struct lustre_sb_info *lsi = s2lsi(sb); - struct obd_device *obd; + struct obd_device *obd = NULL; + struct obd_type *type; int rc = 0; ENTRY; + /* Either an MDT or an OST or neither */ + /* if this was an MDT, and there are no more MDT's, clean up the MDS */ - if (IS_MDT(lsi->lsi_ldd) && (obd = class_name2obd("MDS"))) { + if ((lddflags & LDD_F_SV_TYPE_MDT) && (obd = class_name2obd("MDS"))) { //FIXME pre-rename, should eventually be LUSTRE_MDT_NAME - struct obd_type *type = class_search_type(LUSTRE_MDS_NAME); - if (!type || !type->typ_refcnt) { - /* nobody is using the MDT type, clean the MDS */ - if (lsi->lsi_flags & LSI_UMOUNT_FORCE) - obd->obd_force = 1; - if (lsi->lsi_flags & LSI_UMOUNT_FAILOVER) - obd->obd_fail = 1; - rc = class_manual_cleanup(obd); - } + type = class_search_type(LUSTRE_MDS_NAME); + } + /* if this was an OST, and there are no more OST's, clean up the OSS */ + if ((lddflags & LDD_F_SV_TYPE_OST) && (obd = class_name2obd("OSS"))) { + type = class_search_type(LUSTRE_OST_NAME); } - /* if this was an OST, and there are no more OST's, clean up the OSS */ - if (IS_OST(lsi->lsi_ldd) && (obd = class_name2obd("OSS"))) { - struct obd_type *type = class_search_type(LUSTRE_OST_NAME); - if (!type || !type->typ_refcnt) { - int err; - /* nobody is using the OST type, clean the OSS */ - if (lsi->lsi_flags & LSI_UMOUNT_FORCE) - obd->obd_force = 1; - if (lsi->lsi_flags & LSI_UMOUNT_FAILOVER) - obd->obd_fail = 1; - err = class_manual_cleanup(obd); - if (!rc) - rc = err; - } + if (obd && (!type || !type->typ_refcnt)) { + int err; + if (lsiflags & LSI_UMOUNT_FORCE) + obd->obd_force = 1; + if (lsiflags & LSI_UMOUNT_FAILOVER) + obd->obd_fail = 1; + err = class_manual_cleanup(obd); + if (!rc) + rc = err; } + RETURN(rc); } @@ -1158,14 +1152,39 @@ out_free: RETURN(ERR_PTR(rc)); } +static void server_wait_finished(struct vfsmount *mnt) +{ + wait_queue_head_t waitq; + struct l_wait_info lwi; + int retries = 10; + + init_waitqueue_head(&waitq); + + while ((atomic_read(&mnt->mnt_count) > 0) && retries--) { + CWARN("Mount still busy with %d refs\n", + atomic_read(&mnt->mnt_count)); + + /* Wait for a bit */ + lwi = LWI_TIMEOUT(2 * HZ, NULL, NULL); + l_wait_event(waitq, 0, &lwi); + } + if (atomic_read(&mnt->mnt_count)) { + CERROR("Mount is still busy, giving up.\n"); + } +} + static void server_put_super(struct super_block *sb) { struct lustre_sb_info *lsi = s2lsi(sb); struct obd_device *obd; struct vfsmount *mnt = lsi->lsi_srv_mnt; + int lddflags = lsi->lsi_ldd->ldd_flags; + int lsiflags = lsi->lsi_flags; int rc; ENTRY; + LASSERT(lsiflags & LSI_SERVER); + CDEBUG(D_MOUNT, "server put_super %s\n", lsi->lsi_ldd->ldd_svname); /* Stop the target */ @@ -1192,9 +1211,6 @@ static void server_put_super(struct super_block *sb) } } - /* Stop the servers (MDS, OSS) if no longer needed */ - server_stop_servers(sb); - /* If they wanted the mgs to stop separately from the mdt, they should have put it on a different device. */ if (IS_MGS(lsi->lsi_ldd)) { @@ -1211,6 +1227,15 @@ static void server_put_super(struct super_block *sb) /* drop the One True Mount */ unlock_mntput(mnt); + /* Wait for the targets to really clean up - can't exit (and let the + sb get destroyed) while the mount is still in use */ + server_wait_finished(mnt); + + /* Stop the servers (MDS, OSS) if no longer needed. We must wait + until the target is really gone so that our type refcount check + is right. */ + server_stop_servers(lddflags, lsiflags); + CDEBUG(D_MOUNT, "umount done\n"); EXIT; } -- 1.8.3.1