Whamcloud - gitweb
Branch b1_4_mountconf
authornathan <nathan>
Wed, 22 Feb 2006 00:04:26 +0000 (00:04 +0000)
committernathan <nathan>
Wed, 22 Feb 2006 00:04:26 +0000 (00:04 +0000)
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
lustre/mgs/mgs_handler.c
lustre/mgs/mgs_llog.c
lustre/obdclass/llog_obd.c
lustre/obdclass/obd_mount.c

index c3e7567..95f2e93 100644 (file)
@@ -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);
 }
index 7bccab8..cd65026 100644 (file)
@@ -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);
index 78d3bc1..9fe3af1 100644 (file)
@@ -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);
         }
 
index 0e64454..40d548e 100644 (file)
 
 /* 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;
index 2fa8d31..57d1650 100644 (file)
@@ -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;
 }