From c9ed987a01d6eb22a27b08510bde52db4f17742e Mon Sep 17 00:00:00 2001 From: phil Date: Sat, 12 Feb 2005 21:32:17 +0000 Subject: [PATCH] b=5656 Clean up a lot of calls to LDLM_DEBUG that weren't protected by an ns_lock. This is probably not a comprehensive fix, but addresses all of the violations that I saw. --- lustre/ldlm/ldlm_lock.c | 19 +++++++++---------- lustre/ldlm/ldlm_lockd.c | 20 ++++++++++++++++++-- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/lustre/ldlm/ldlm_lock.c b/lustre/ldlm/ldlm_lock.c index 9e3add6..ad3cb9a 100644 --- a/lustre/ldlm/ldlm_lock.c +++ b/lustre/ldlm/ldlm_lock.c @@ -436,9 +436,9 @@ void ldlm_lock_addref_internal(struct ldlm_lock *lock, __u32 mode) if (mode & (LCK_EX | LCK_CW | LCK_PW)) lock->l_writers++; lock->l_last_used = jiffies; - l_unlock(&lock->l_resource->lr_namespace->ns_lock); LDLM_LOCK_GET(lock); LDLM_DEBUG(lock, "ldlm_lock_addref(%s)", ldlm_lockname[mode]); + l_unlock(&lock->l_resource->lr_namespace->ns_lock); } void ldlm_lock_decref_internal(struct ldlm_lock *lock, __u32 mode) @@ -446,9 +446,9 @@ void ldlm_lock_decref_internal(struct ldlm_lock *lock, __u32 mode) struct ldlm_namespace *ns; ENTRY; - LDLM_DEBUG(lock, "ldlm_lock_decref(%s)", ldlm_lockname[mode]); ns = lock->l_resource->lr_namespace; l_lock(&ns->ns_lock); + LDLM_DEBUG(lock, "ldlm_lock_decref(%s)", ldlm_lockname[mode]); if (mode & (LCK_NL | LCK_CR | LCK_PR)) { LASSERT(lock->l_readers > 0); lock->l_readers--; @@ -518,8 +518,8 @@ void ldlm_lock_decref_and_cancel(struct lustre_handle *lockh, __u32 mode) LASSERT(lock != NULL); - LDLM_DEBUG(lock, "ldlm_lock_decref(%s)", ldlm_lockname[mode]); l_lock(&lock->l_resource->lr_namespace->ns_lock); + LDLM_DEBUG(lock, "ldlm_lock_decref(%s)", ldlm_lockname[mode]); lock->l_flags |= LDLM_FL_CBPENDING; l_unlock(&lock->l_resource->lr_namespace->ns_lock); ldlm_lock_decref_internal(lock, mode); @@ -694,13 +694,15 @@ int ldlm_lock_match(struct ldlm_namespace *ns, int flags, (lock->l_flags & LDLM_FL_CAN_MATCH), &lwi); } } - if (rc) + if (rc) { + l_lock(&ns->ns_lock); LDLM_DEBUG(lock, "matched ("LPU64" "LPU64")", type == LDLM_PLAIN ? res_id->name[2] : policy->l_extent.start, type == LDLM_PLAIN ? res_id->name[3] : policy->l_extent.end); - else if (!(flags & LDLM_FL_TEST_LOCK)) /* less verbose for test-only */ + l_unlock(&ns->ns_lock); + } else if (!(flags & LDLM_FL_TEST_LOCK)) { /*less verbose for test-only*/ LDLM_DEBUG_NOLOCK("not matched ns %p type %u mode %u res " LPU64"/"LPU64" ("LPU64" "LPU64")", ns, type, mode, res_id->name[0], res_id->name[1], @@ -708,7 +710,7 @@ int ldlm_lock_match(struct ldlm_namespace *ns, int flags, policy->l_extent.start, type == LDLM_PLAIN ? res_id->name[3] : policy->l_extent.end); - + } if (old_lock) LDLM_LOCK_PUT(old_lock); if (flags & LDLM_FL_TEST_LOCK && rc) @@ -1020,15 +1022,12 @@ void ldlm_lock_cancel(struct ldlm_lock *lock) struct ldlm_namespace *ns; ENTRY; - /* There's no race between calling this and taking the ns lock below; - * a lock can only be put on the waiting list once, because it can only - * issue a blocking AST once. */ + l_lock(&ns->ns_lock); ldlm_del_waiting_lock(lock); res = lock->l_resource; ns = res->lr_namespace; - l_lock(&ns->ns_lock); /* Please do not, no matter how tempting, remove this LBUG without * talking to me first. -phik */ if (lock->l_readers || lock->l_writers) { diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index b3e11ce0..214e9df 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -223,11 +223,15 @@ static void waiting_locks_callback(unsigned long unused) * lock. We add it to the pending-callback chain, and schedule the lock-timeout * timer to fire appropriately. (We round up to the next second, to avoid * floods of timer firings during periods of high lock contention and traffic). + * + * Called with the namespace lock held. */ static int ldlm_add_waiting_lock(struct ldlm_lock *lock) { unsigned long timeout_rounded; + l_check_ns_lock(lock->l_resource->lr_namespace); + spin_lock_bh(&waiting_locks_spinlock); if (!list_empty(&lock->l_pending_chain)) { spin_unlock_bh(&waiting_locks_spinlock); @@ -253,11 +257,15 @@ static int ldlm_add_waiting_lock(struct ldlm_lock *lock) * Remove a lock from the pending list, likely because it had its cancellation * callback arrive without incident. This adjusts the lock-timeout timer if * needed. Returns 0 if the lock wasn't pending after all, 1 if it was. + * + * Called with namespace lock held. */ int ldlm_del_waiting_lock(struct ldlm_lock *lock) { struct list_head *list_next; + l_check_ns_lock(lock->l_resource->lr_namespace); + if (lock->l_export == NULL) { /* We don't have a "waiting locks list" on clients. */ LDLM_DEBUG(lock, "client lock: no-op"); @@ -379,8 +387,8 @@ int ldlm_server_blocking_ast(struct ldlm_lock *lock, if (lock->l_granted_mode != lock->l_req_mode) { /* this blocking AST will be communicated as part of the * completion AST instead */ - l_unlock(&lock->l_resource->lr_namespace->ns_lock); LDLM_DEBUG(lock, "lock not granted, not sending blocking AST"); + l_unlock(&lock->l_resource->lr_namespace->ns_lock); RETURN(0); } @@ -696,8 +704,10 @@ existing_lock: /* The LOCK_CHANGED code in ldlm_lock_enqueue depends on this * ldlm_reprocess_all. If this moves, revisit that code. -phil */ if (lock) { + l_lock(&lock->l_resource->lr_namespace->ns_lock); LDLM_DEBUG(lock, "server-side enqueue handler, sending reply" "(err=%d, rc=%d)", err, rc); + l_unlock(&lock->l_resource->lr_namespace->ns_lock); if (rc == 0) { down(&lock->l_resource->lr_lvb_sem); @@ -804,8 +814,10 @@ int ldlm_handle_cancel(struct ptlrpc_request *req) dlm_req->lock_handle1.cookie); req->rq_status = ESTALE; } else { - LDLM_DEBUG(lock, "server-side cancel handler START"); res = lock->l_resource; + l_lock(&res->lr_namespace->ns_lock); + LDLM_DEBUG(lock, "server-side cancel handler START"); + l_unlock(&res->lr_namespace->ns_lock); if (res && res->lr_namespace->ns_lvbo && res->lr_namespace->ns_lvbo->lvbo_update) { (void)res->lr_namespace->ns_lvbo->lvbo_update @@ -813,9 +825,11 @@ int ldlm_handle_cancel(struct ptlrpc_request *req) //(res, req->rq_reqmsg, 1); } + l_lock(&res->lr_namespace->ns_lock); ldlm_lock_cancel(lock); if (ldlm_del_waiting_lock(lock)) CDEBUG(D_DLMTRACE, "cancelled waiting lock %p\n", lock); + l_unlock(&res->lr_namespace->ns_lock); req->rq_status = rc; } @@ -824,7 +838,9 @@ int ldlm_handle_cancel(struct ptlrpc_request *req) if (lock) { ldlm_reprocess_all(lock->l_resource); + l_lock(&lock->l_resource->lr_namespace->ns_lock); LDLM_DEBUG(lock, "server-side cancel handler END"); + l_unlock(&lock->l_resource->lr_namespace->ns_lock); LDLM_LOCK_PUT(lock); } -- 1.8.3.1