b=9942,9903

r=adilger

Treat DISP_ENQ_COMPLETE as a flag that request referenced from this intent
has extra reference that needs to be released, if still set at intent freeing
time.

diff --git a/lustre/ChangeLog b/lustre/ChangeLog
index 6a1e371..0aa1482 100644 (file)
--- a/lustre/ChangeLog
+++ b/lustre/ChangeLog
@@ -464,6 +464,18 @@ Description: Inode refcounting problems in NFS export code
 Details    : link_raw functions used to call d_instantiate without obtaining
             extra inode reference first.
 
+Severity   : minor
+Frequency  : rare
+Bugzilla   : 9942,9903
+Description: Referencing freed requests leading to crash, memleask with NFS.
+Details    : We used to require that call to ll_revalidate_it was always
+            followed by ll_lookup_it. Also with revalidate_special() it is
+            possible to call ll_revalidate_it() twice for the same dentry
+            even if first occurence returned success. This fix changes semantic
+            between DISP_ENQ_COMPLETE disposition flag to mean there is extra
+            reference on a request referred from the intent.
+            ll_intent_release() then releases such a request.
+
 ------------------------------------------------------------------------------
 
 08-26-2005  Cluster File Systems, Inc. <info@clusterfs.com>

diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c
index 793c657..106711d 100644 (file)
--- a/lustre/llite/dcache.c
+++ b/lustre/llite/dcache.c
@@ -134,6 +134,12 @@ void ll_intent_release(struct lookup_intent *it)
         ll_intent_drop_lock(it);
         it->it_magic = 0;
         it->it_op_release = 0;
+        if (it_disposition(it, DISP_ENQ_COMPLETE)) {
+                /* We are still holding extra reference on a request, need to
+                   free it */
+                struct ptlrpc_request *request = it->d.lustre.it_data;
+                ptlrpc_req_finished(request);
+        }
         it->d.lustre.it_disposition = 0;
         it->d.lustre.it_data = NULL;
         EXIT;
@@ -337,11 +343,11 @@ int ll_revalidate_it(struct dentry *de, int lookup_flags,
         spin_unlock(&dcache_lock);
 
  out:
-        /* If we had succesful it lookup on mds, but it happened to be negative,
-           we do not free request as it will be reused during lookup (see
-           comment in mdc/mdc_locks.c::mdc_intent_lock(). But if
+        /* We do not free request as it may be reused during following lookup
+          (see comment in mdc/mdc_locks.c::mdc_intent_lock()), request will
+           be freed in ll_lookup_it or in ll_intent_release. But if
            request was not completed, we need to free it. (bug 5154) */
-        if (req != NULL && (rc == 1 || !it_disposition(it, DISP_ENQ_COMPLETE)))
+        if (req != NULL && !it_disposition(it, DISP_ENQ_COMPLETE))
                 ptlrpc_req_finished(req);
         if (rc == 0) {
                 ll_unhash_aliases(de->d_inode);

diff --git a/lustre/mdc/mdc_locks.c b/lustre/mdc/mdc_locks.c
index 9c073cd..080d670 100644 (file)
--- a/lustre/mdc/mdc_locks.c
+++ b/lustre/mdc/mdc_locks.c
@@ -54,6 +54,11 @@ void it_set_disposition(struct lookup_intent *it, int flag)
 }
 EXPORT_SYMBOL(it_set_disposition);
 
+void it_clear_disposition(struct lookup_intent *it, int flag)
+{
+        it->d.lustre.it_disposition &= ~flag;
+}
+
 static int it_to_lock_mode(struct lookup_intent *it)
 {
         /* CREAT needs to be tested before open (both could be set) */
@@ -593,6 +598,12 @@ int mdc_intent_lock(struct obd_export *exp, struct mdc_op_data *op_data,
                 if (rc < 0)
                         RETURN(rc);
                 memcpy(&it->d.lustre.it_lock_handle, &lockh, sizeof(lockh));
+        } else if (!op_data->fid2.id) {
+                /* DISP_ENQ_COMPLETE set means there is extra reference on
+                   request referenced from this intent, saved for subsequent
+                   lookup. This path is executed when we proceed to this
+                   lookup, so we clear DISP_ENQ_COMPLETE */
+                it_clear_disposition(it, DISP_ENQ_COMPLETE);
         }
         request = *reqp = it->d.lustre.it_data;
         LASSERT(request != NULL);