Drop the filter export grants under the same lock as the removal of the

export from the device from the export list.  This still isn't possible
for --force disconnects, so we don't do filter_grant_sanity_check() at
that time.

Also, change the LASSERTs to CERRORs in case this happens again.  First of
all to see if these errors are just transient during export disconnection
which means they are just overly-aggressive assertions, or if there is a
permanent accounting error that indicates some other problem.  Secondly, it
isn't possible to stop the use of existing export references, so there will
always be some small race during client eviction as completing operations
might update the export before it is destroyed.
b=2663

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 4a27abb..f5107a5 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -3,6 +3,7 @@ tbd  Cluster File Systems, Inc. <info@clusterfs.com>
        * bug fixes
        - fixes for glimpse AST timeouts / incorrectly 0-sized files (2818)
        - don't overwrite extent policy data in reply if lock was blocked (2901)
+       - drop filter export grants atomically with removal from device (2663)
 
 2004-03-04  Cluster File Systems, Inc. <info@clusterfs.com>
        * version 1.2.0

diff --git a/lustre/obdfilter/filter.c b/lustre/obdfilter/filter.c
index e90fb45..7c8f456 100644 (file)
--- a/lustre/obdfilter/filter.c
+++ b/lustre/obdfilter/filter.c
@@ -1450,23 +1450,23 @@ static void filter_grant_sanity_check(struct obd_device *obd, char *func)
         spin_unlock(&obd->obd_osfs_lock);
 
         /* Do these assertions outside the spinlocks so we don't kill system */
-        LASSERTF(tot_granted == fo_tot_granted, "%s "LPU64" != "LPU64"\n",
-                 func, tot_granted, fo_tot_granted);
-        LASSERTF(tot_pending == fo_tot_pending, "%s "LPU64" != "LPU64"\n",
-                 func, tot_pending, fo_tot_pending);
-        LASSERTF(tot_dirty == fo_tot_dirty, "%s "LPU64" != "LPU64"\n",
-                 func, tot_dirty, fo_tot_dirty);
-        LASSERTF(tot_pending <= tot_granted, "%s "LPU64" > "LPU64"\n",
-                 func, tot_pending, tot_granted);
-        LASSERTF(tot_granted <= maxsize, "%s "LPU64" > "LPU64"\n",
-                 func, tot_granted, maxsize);
-        LASSERTF(tot_dirty <= maxsize, "%s "LPU64" > "LPU64"\n",
-                 func, tot_dirty, maxsize);
+        CERROR("%s: tot_granted "LPU64" != fo_tot_granted "LPU64"\n",
+               func, tot_granted, fo_tot_granted);
+        CERROR("%s: tot_pending "LPU64" != fo_tot_pending "LPU64"\n",
+               func, tot_pending, fo_tot_pending);
+        CERROR("%s: tot_dirty "LPU64" != fo_tot_dirty "LPU64"\n",
+               func, tot_dirty, fo_tot_dirty);
+        CERROR("%s: tot_pending "LPU64" > tot_granted "LPU64"\n",
+               func, tot_pending, tot_granted);
+        CERROR("%s: tot_granted "LPU64" > maxsize "LPU64"\n",
+               func, tot_granted, maxsize);
+        CERROR("%s: tot_dirty "LPU64" > maxsize "LPU64"\n",
+               func, tot_dirty, maxsize);
 }
 
-/* Remove this client from the grant accounting totals.  This is done at
- * disconnect time and also at export destroy time in case there was a race
- * between removing the export and an incoming BRW updating the client grant.
+/* Remove this client from the grant accounting totals.  We also remove
+ * the export from the obd device under the osfs and dev locks to ensure
+ * that the filter_grant_sanity_check() calculations are always valid.
  * The client should do something similar when it invalidates its import. */
 static void filter_grant_discard(struct obd_export *exp)
 {
@@ -1475,6 +1475,10 @@ static void filter_grant_discard(struct obd_export *exp)
         struct filter_export_data *fed = &exp->exp_filter_data;
 
         spin_lock(&obd->obd_osfs_lock);
+        spin_lock(&exp->exp_obd->obd_dev_lock);
+        list_del_init(&exp->exp_obd_chain);
+        spin_unlock(&exp->exp_obd->obd_dev_lock);
+
         CDEBUG(D_CACHE, "%s: cli %s/%p dirty %lu pend %lu grant %lu\n",
                obd->obd_name, exp->exp_client_uuid.uuid, exp,
                fed->fed_dirty, fed->fed_pending, fed->fed_grant);
@@ -1513,7 +1517,9 @@ static int filter_destroy_export(struct obd_export *exp)
         if (exp->exp_obd->obd_replayable)
                 filter_client_free(exp, exp->exp_flags);
 
-        filter_grant_sanity_check(exp->exp_obd, __FUNCTION__);
+        filter_grant_discard(exp);
+        if (!(exp->exp_flags & OBD_OPT_FORCE))
+                filter_grant_sanity_check(exp->exp_obd, __FUNCTION__);
 
         RETURN(0);
 }
@@ -1534,15 +1540,13 @@ static int filter_disconnect(struct obd_export *exp, int flags)
         exp->exp_flags = flags;
         spin_unlock_irqrestore(&exp->exp_lock, irqflags);
 
+        if (!(flags & OBD_OPT_FORCE))
+                filter_grant_sanity_check(obd, __FUNCTION__);
         filter_grant_discard(exp);
 
         /* Disconnect early so that clients can't keep using export */
         rc = class_disconnect(exp, flags);
 
-        /* Do this twice in case a BRW arrived between the first call and
-         * the class_export_unlink() call (bug 2663) */
-        filter_grant_discard(exp);
-
         ldlm_cancel_locks_for_export(exp);
 
         fsfilt_sync(obd, obd->u.filter.fo_sb);