From 29c6c9f2ebaa73da00cb0786e4579c2cecca068a Mon Sep 17 00:00:00 2001 From: pschwan Date: Mon, 17 Jun 2002 22:29:04 +0000 Subject: [PATCH] - Fixed some screwy ldlm resource refcount issues --- lustre/include/linux/lustre_dlm.h | 2 +- lustre/ldlm/ldlm_lock.c | 10 +++++++--- lustre/ldlm/ldlm_lockd.c | 16 +++++++++++++--- lustre/ldlm/ldlm_resource.c | 5 +++-- lustre/mds/mds_reint.c | 7 ++++--- 5 files changed, 28 insertions(+), 12 deletions(-) diff --git a/lustre/include/linux/lustre_dlm.h b/lustre/include/linux/lustre_dlm.h index 875be13..0eb64d5 100644 --- a/lustre/include/linux/lustre_dlm.h +++ b/lustre/include/linux/lustre_dlm.h @@ -241,7 +241,7 @@ struct ldlm_resource *ldlm_resource_get(struct ldlm_namespace *ns, int ldlm_resource_put(struct ldlm_resource *res); void ldlm_resource_add_lock(struct ldlm_resource *res, struct list_head *head, struct ldlm_lock *lock); -void ldlm_resource_del_lock(struct ldlm_lock *lock); +int ldlm_resource_del_lock(struct ldlm_lock *lock); void ldlm_res2desc(struct ldlm_resource *res, struct ldlm_resource_desc *desc); void ldlm_resource_dump(struct ldlm_resource *res); diff --git a/lustre/ldlm/ldlm_lock.c b/lustre/ldlm/ldlm_lock.c index 3ff3c7e..94df819 100644 --- a/lustre/ldlm/ldlm_lock.c +++ b/lustre/ldlm/ldlm_lock.c @@ -532,7 +532,7 @@ ldlm_error_t ldlm_local_lock_enqueue(struct lustre_handle *lockh, /* The server returned a blocked lock, but it was granted before * we got a chance to actually enqueue it. We don't need to do * anything else. */ - GOTO(out, ELDLM_OK); + GOTO(out_noput, ELDLM_OK); } /* If this is a local resource, put it on the appropriate list. */ @@ -569,6 +569,11 @@ ldlm_error_t ldlm_local_lock_enqueue(struct lustre_handle *lockh, ldlm_grant_lock(res, lock); EXIT; out: + /* We're called with a lock that has a referenced resource and is not on + * any resource list. When we added it to a list, we incurred an extra + * reference. */ + ldlm_resource_put(lock->l_resource); + out_noput: /* Don't set 'completion_ast' until here so that if the lock is granted * immediately we don't do an unnecessary completion call. */ lock->l_completion_ast = completion; @@ -654,8 +659,7 @@ struct ldlm_resource *ldlm_local_lock_cancel(struct ldlm_lock *lock) CDEBUG(D_INFO, "lock still has references (%d readers, %d " "writers)\n", lock->l_readers, lock->l_writers); - ldlm_resource_del_lock(lock); - if (ldlm_resource_put(res)) + if (ldlm_resource_del_lock(lock)) res = NULL; /* res was freed, nothing else to do. */ else spin_unlock(&res->lr_lock); diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index 3e1cc75..d5e2dfe 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -41,8 +41,18 @@ static int common_callback(struct ldlm_lock *lock, struct ldlm_lock *new, if (!new) { CDEBUG(D_INFO, "Got local completion AST for lock %p.\n", lock); lock->l_req_mode = mode; - list_del_init(&lock->l_res_link); + + /* FIXME: the API is flawed if I have to do these refcount + * acrobatics (along with the _put() below). */ + lock->l_resource->lr_refcount++; + + /* _del_lock is safe for half-created locks that are not yet on + * a list. */ + ldlm_resource_del_lock(lock); ldlm_grant_lock(lock->l_resource, lock); + + ldlm_resource_put(lock->l_resource); + wake_up(&lock->l_waitq); spin_unlock(&lock->l_lock); spin_unlock(&lock->l_resource->lr_lock); @@ -237,8 +247,8 @@ static int _ldlm_callback(struct ptlrpc_service *svc, common_callback(lock1, lock2, dlm_req->lock_desc.l_granted_mode, NULL, 0, NULL); - LDLM_DEBUG_NOLOCK("client %s callback handler END", - lock2 == NULL ? "completion" : "blocked"); + LDLM_DEBUG_NOLOCK("client %s callback handler END (lock: %p)", + lock2 == NULL ? "completion" : "blocked", lock1); RETURN(0); } diff --git a/lustre/ldlm/ldlm_resource.c b/lustre/ldlm/ldlm_resource.c index 67fdf88..ebda971 100644 --- a/lustre/ldlm/ldlm_resource.c +++ b/lustre/ldlm/ldlm_resource.c @@ -317,12 +317,13 @@ void ldlm_resource_add_lock(struct ldlm_resource *res, struct list_head *head, } /* Must be called with resource->lr_lock taken */ -void ldlm_resource_del_lock(struct ldlm_lock *lock) +int ldlm_resource_del_lock(struct ldlm_lock *lock) { if (!list_empty(&lock->l_res_link)) { list_del_init(&lock->l_res_link); - lock->l_resource->lr_refcount--; + return ldlm_resource_put(lock->l_resource); } + return 0; } int ldlm_get_resource_handle(struct ldlm_resource *res, struct lustre_handle *h) diff --git a/lustre/mds/mds_reint.c b/lustre/mds/mds_reint.c index b2ab3c3..666dacd 100644 --- a/lustre/mds/mds_reint.c +++ b/lustre/mds/mds_reint.c @@ -513,9 +513,10 @@ out_unlink_de: if (!rc) { lock = lustre_handle2object(&lockh); ldlm_lock_decref(lock, LCK_EX); - if (ldlm_cli_cancel(lock->l_client, lock)) - CERROR("failed to get child inode lock ino %Ld\n", - res_id[0]); + rc = ldlm_cli_cancel(lock->l_client, lock); + if (rc < 0) + CERROR("failed to cancel child inode lock ino " + "%Ld: %d\n", res_id[0], rc); } out_unlink: -- 1.8.3.1