Skip to content

Commit

Permalink
ocfs2: do not modify lksb->status in the unlock ast
Browse files Browse the repository at this point in the history
This can race with other ast notification, which can cause bad status values
to propagate into the unlock ast.

Signed-off-by: Kurt Hackel <kurt.hackel@oracle.com>
Signed-off-by: Mark Fasheh <mark.fasheh@oracle.com>
  • Loading branch information
Kurt Hackel authored and Mark Fasheh committed Aug 7, 2006
1 parent 4b1af77 commit a23eac9
Showing 1 changed file with 12 additions and 25 deletions.
37 changes: 12 additions & 25 deletions fs/ocfs2/dlm/dlmunlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm,
else
status = dlm_get_unlock_actions(dlm, res, lock, lksb, &actions);

if (status != DLM_NORMAL)
if (status != DLM_NORMAL && status != DLM_CANCELGRANT)
goto leave;

/* By now this has been masked out of cancel requests. */
Expand Down Expand Up @@ -183,8 +183,7 @@ static enum dlm_status dlmunlock_common(struct dlm_ctxt *dlm,
spin_lock(&lock->spinlock);
/* if the master told us the lock was already granted,
* let the ast handle all of these actions */
if (status == DLM_NORMAL &&
lksb->status == DLM_CANCELGRANT) {
if (status == DLM_CANCELGRANT) {
actions &= ~(DLM_UNLOCK_REMOVE_LOCK|
DLM_UNLOCK_REGRANT_LOCK|
DLM_UNLOCK_CLEAR_CONVERT_TYPE);
Expand Down Expand Up @@ -349,14 +348,9 @@ static enum dlm_status dlm_send_remote_unlock_request(struct dlm_ctxt *dlm,
vec, veclen, owner, &status);
if (tmpret >= 0) {
// successfully sent and received
if (status == DLM_CANCELGRANT)
ret = DLM_NORMAL;
else if (status == DLM_FORWARD) {
if (status == DLM_FORWARD)
mlog(0, "master was in-progress. retry\n");
ret = DLM_FORWARD;
} else
ret = status;
lksb->status = status;
ret = status;
} else {
mlog_errno(tmpret);
if (dlm_is_host_down(tmpret)) {
Expand All @@ -372,7 +366,6 @@ static enum dlm_status dlm_send_remote_unlock_request(struct dlm_ctxt *dlm,
/* something bad. this will BUG in ocfs2 */
ret = dlm_err_to_dlm_status(tmpret);
}
lksb->status = ret;
}

return ret;
Expand Down Expand Up @@ -511,11 +504,8 @@ int dlm_unlock_lock_handler(struct o2net_msg *msg, u32 len, void *data)
"cookie=%u:%llu\n",
dlm_get_lock_cookie_node(unlock->cookie),
dlm_get_lock_cookie_seq(unlock->cookie));
else {
/* send the lksb->status back to the other node */
status = lksb->status;
else
dlm_lock_put(lock);
}

leave:
if (res)
Expand All @@ -537,26 +527,22 @@ static enum dlm_status dlm_get_cancel_actions(struct dlm_ctxt *dlm,

if (dlm_lock_on_list(&res->blocked, lock)) {
/* cancel this outright */
lksb->status = DLM_NORMAL;
status = DLM_NORMAL;
*actions = (DLM_UNLOCK_CALL_AST |
DLM_UNLOCK_REMOVE_LOCK);
} else if (dlm_lock_on_list(&res->converting, lock)) {
/* cancel the request, put back on granted */
lksb->status = DLM_NORMAL;
status = DLM_NORMAL;
*actions = (DLM_UNLOCK_CALL_AST |
DLM_UNLOCK_REMOVE_LOCK |
DLM_UNLOCK_REGRANT_LOCK |
DLM_UNLOCK_CLEAR_CONVERT_TYPE);
} else if (dlm_lock_on_list(&res->granted, lock)) {
/* too late, already granted. DLM_CANCELGRANT */
lksb->status = DLM_CANCELGRANT;
status = DLM_NORMAL;
/* too late, already granted. */
status = DLM_CANCELGRANT;
*actions = DLM_UNLOCK_CALL_AST;
} else {
mlog(ML_ERROR, "lock to cancel is not on any list!\n");
lksb->status = DLM_IVLOCKID;
status = DLM_IVLOCKID;
*actions = 0;
}
Expand All @@ -573,13 +559,11 @@ static enum dlm_status dlm_get_unlock_actions(struct dlm_ctxt *dlm,

/* unlock request */
if (!dlm_lock_on_list(&res->granted, lock)) {
lksb->status = DLM_DENIED;
status = DLM_DENIED;
dlm_error(status);
*actions = 0;
} else {
/* unlock granted lock */
lksb->status = DLM_NORMAL;
status = DLM_NORMAL;
*actions = (DLM_UNLOCK_FREE_LOCK |
DLM_UNLOCK_CALL_AST |
Expand Down Expand Up @@ -671,7 +655,7 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb,
}

if (call_ast) {
mlog(0, "calling unlockast(%p, %d)\n", data, lksb->status);
mlog(0, "calling unlockast(%p, %d)\n", data, status);
if (is_master) {
/* it is possible that there is one last bast
* pending. make sure it is flushed, then
Expand All @@ -683,9 +667,12 @@ enum dlm_status dlmunlock(struct dlm_ctxt *dlm, struct dlm_lockstatus *lksb,
wait_event(dlm->ast_wq,
dlm_lock_basts_flushed(dlm, lock));
}
(*unlockast)(data, lksb->status);
(*unlockast)(data, status);
}

if (status == DLM_CANCELGRANT)
status = DLM_NORMAL;

if (status == DLM_NORMAL) {
mlog(0, "kicking the thread\n");
dlm_kick_thread(dlm, res);
Expand Down

0 comments on commit a23eac9

Please sign in to comment.