Whamcloud - gitweb
again fix the intent release problem in dentry revalidate path,
authorericm <ericm>
Thu, 12 May 2005 00:56:59 +0000 (00:56 +0000)
committerericm <ericm>
Thu, 12 May 2005 00:56:59 +0000 (00:56 +0000)
simplified/rewrite some logic in ll_revalidate_it(), pls back this
out if any problem found.

lustre/llite/dcache.c

index 00da5d0..87a828d 100644 (file)
@@ -353,57 +353,11 @@ int ll_revalidate_it(struct dentry *de, int flags, struct nameidata *nd,
         OBD_FAIL_TIMEOUT(OBD_FAIL_MDC_REVALIDATE_PAUSE, 5);
 
         gns_it = it ? it->it_op : IT_OPEN;
-        ll_frob_intent(&it, &lookup_it);
-
-        LASSERT(it != NULL);
-
-        if (it->it_op == IT_GETATTR) { /* We need to check for LOOKUP lock as
-                                          well */
-                rc = ll_intent_alloc(&lookup_it);
-                if (rc)
-                        LBUG(); /* Can't think of better idea just yet */
-
-                rc = md_intent_lock(exp, &pid, de->d_name.name,
-                                    de->d_name.len, NULL, 0, &cid, &lookup_it,
-                                    flags, &req, ll_mdc_blocking_ast);
-                /* If there was no lookup lock, no point in even checking for
-                   UPDATE lock */
-                if (!rc) {
-                        /* 
-                         * freeing @it allocated in ll_frob_intent() above in
-                         * this function before replacing @it by @lookup_it and
-                         * thus lossing it.  --umka
-                         */
-                        ll_intent_free(it);
-                        it = &lookup_it;
-                        if (!req) {
-                                ll_intent_free(it);
-                                goto do_lookup;
-                        }
-                        GOTO(out, rc);
-                }
-                if (it_disposition(&lookup_it, DISP_LOOKUP_NEG)) {
-                        /* 
-                         * freeing @it allocated in ll_frob_intent() above in
-                         * this function before replacing @it by @lookup_it and
-                         * thus lossing it.  --umka
-                         */
-                        ll_intent_free(it);
-                        it = &lookup_it;
-                        ll_intent_free(it);
-                        GOTO(out, rc = 0);
-                }
-
-                if (req)
-                        ptlrpc_req_finished(req);
-                req = NULL;
-                ll_lookup_finish_locks(&lookup_it, de);
-                /* XXX: on 2.6 ll_lookup_finish_locks does not call ll_intent_release */
-                ll_intent_release(&lookup_it);
-        }
 
-        /* open lock stuff */
-        if ((it->it_op == IT_OPEN) && de->d_inode) {
+        if (it && it->it_op == IT_GETATTR)
+                it = NULL; /* will use it_lookup */
+        else if (it && (it->it_op == IT_OPEN) && de->d_inode) {
+                /* open lock stuff */
                 struct inode *inode = de->d_inode;
                 struct ll_inode_info *lli = ll_i2info(inode);
                 struct obd_client_handle **och_p;
@@ -447,6 +401,8 @@ int ll_revalidate_it(struct dentry *de, int flags, struct nameidata *nd,
                            if it would be, we'll reopen the open request to
                            MDS later during file open path */
                         up(&lli->lli_och_sem);
+                        if (ll_intent_alloc(it))
+                                LBUG();
                         memcpy(&LUSTRE_IT(it)->it_lock_handle, &lockh,
                                sizeof(lockh));
                         LUSTRE_IT(it)->it_lock_mode = lockmode;
@@ -466,6 +422,9 @@ int ll_revalidate_it(struct dentry *de, int flags, struct nameidata *nd,
         }
 
 do_lock:
+        ll_frob_intent(&it, &lookup_it);
+        LASSERT(it != NULL);
+
         rc = md_intent_lock(exp, &pid, de->d_name.name, de->d_name.len,
                             NULL, 0, &cid, it, flags, &req, ll_mdc_blocking_ast);
         /* If req is NULL, then md_intent_lock() only tried to do a lock match;
@@ -510,26 +469,11 @@ out:
         }
 
         if (rc == 0) {
-                if (it == &lookup_it) {
-                        /* 
-                         * releasing intent with cloberring ->magic etc. as this
-                         * is our @lookup_it which will not be used out of this
-                         * function.  --umka
-                         */
+                if (it == &lookup_it)
                         ll_intent_release(it);
-                } else {
-                        /* 
-                         * as dentry is not revalidated, ll_llokup_it() me be
-                         * called. Thus we should make sure that lock is dropped
-                         * and intent freed without clobbering ->magic, etc. We
-                         * free intent allocated in ll__frob_intent() called in
-                         * this function.  --umka
-                         */
-                        ll_intent_drop_lock(it);
-                        ll_intent_free(it);
-                }
+
                 ll_unhash_aliases(de->d_inode);
-                return 0;
+                RETURN(0);
         }
 
         /* 
@@ -539,19 +483,19 @@ out:
          * lookup control path, which is always made with parent's i_sem taken.
          * --umka
          */
-        if (rc && atomic_read(&ll_i2sbi(de->d_inode)->ll_gns_enabled) &&
-            !(!((de->d_inode->i_mode & S_ISUID) && S_ISDIR(de->d_inode->i_mode)) ||
-              !(flags & LOOKUP_CONTINUE || (gns_it & (IT_CHDIR | IT_OPEN))))) {
+        if (atomic_read(&ll_i2sbi(de->d_inode)->ll_gns_enabled) &&
+            (de->d_inode->i_mode & S_ISUID) && S_ISDIR(de->d_inode->i_mode) &&
+            (flags & LOOKUP_CONTINUE || (gns_it & (IT_CHDIR | IT_OPEN)))) {
                 /* 
                  * special "." and ".." has to be always revalidated because
                  * they never should be passed to lookup()
                  */
                 if (!ll_special_name(de)) {
-                        /* 
-                         * releasing intent for lookup case as @it in this time
-                         * our private it and will not be used anymore in this
-                         * control path.  --umka
+                        /* XXX umka: if req not NULL we might need free
+                         *     the req we already obtianed?
                          */
+                        LASSERT(req == NULL);
+
                         if (it == &lookup_it) {
                                 ll_intent_release(it);
                         } else {
@@ -565,7 +509,7 @@ out:
                                 ll_intent_free(it);
                         }
                         ll_unhash_aliases(de->d_inode);
-                        return 0;
+                        RETURN (0);
                 }
         }
 
@@ -574,22 +518,21 @@ out:
                de->d_name.name, de, de->d_parent, de->d_inode,
                atomic_read(&de->d_count));
 
-        ll_lookup_finish_locks(it, de);
-        de->d_flags &= ~DCACHE_LUSTRE_INVALID;
+        if (it == &lookup_it)
+                ll_intent_release(it);
+        else
+                ll_lookup_finish_locks(it, de);
 
-        /* 
-         * here @it should be released in both cases, as in the case @it is not
-         * @lookup_it we release intent allocated in ll_frob_intent(). Here we
-         * can use ll_intent_release() which also clobers ->magic as dentry is
-         * revalidated and this intent will not be passed to ll_lookup_it() and
-         * will not confuse it.  --umka
-         */
-        ll_intent_release(it);
+        de->d_flags &= ~DCACHE_LUSTRE_INVALID;
         return rc;
+
 do_lookup:
-        it = &lookup_it;
-        if (ll_intent_alloc(it))
-                LBUG();
+        if (it != &lookup_it) {
+                ll_intent_release(it);
+                it = &lookup_it;
+                if (ll_intent_alloc(it))
+                        LBUG();
+        }
         
         // We did that already, right?  ll_inode2id(&pid, de->d_parent->d_inode);
         rc = md_intent_lock(exp, &pid, de->d_name.name,