From 6a898be4605581044366c2c9ee2b5e92b0662599 Mon Sep 17 00:00:00 2001 From: ericm Date: Thu, 12 May 2005 00:56:59 +0000 Subject: [PATCH] again fix the intent release problem in dentry revalidate path, simplified/rewrite some logic in ll_revalidate_it(), pls back this out if any problem found. --- lustre/llite/dcache.c | 121 +++++++++++++------------------------------------- 1 file changed, 32 insertions(+), 89 deletions(-) diff --git a/lustre/llite/dcache.c b/lustre/llite/dcache.c index 00da5d0..87a828d 100644 --- a/lustre/llite/dcache.c +++ b/lustre/llite/dcache.c @@ -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, -- 1.8.3.1