[Ocfs2-devel] [PATCH] ocfs2/dlm: fix race between convert and recovery

[Tariq Saeed]

On 09/17/2015 06:17 AM, Joseph Qi wrote:
> There is a race window between dlmconvert_remote and
> dlm_move_lockres_to_recovery_list, which will cause a lock with
> OCFS2_LOCK_BUSY in grant list, thus system hangs.
>
> dlmconvert_remote
> {
> 	spin_lock(&res->spinlock);
> 	list_move_tail(&lock->list, &res->converting);
> 	lock->convert_pending = 1;
> 	spin_unlock(&res->spinlock);
>
> 	status = dlm_send_remote_convert_request();
> 	>>>>>> race window, master has queued ast and return DLM_NORMAL,
> 	       and then down before sending ast.
> 	       this node detects master down and call
> 	       dlm_move_lockres_to_recovery_list, which will revert the
> 	       lock to grant list.
> 	       Then OCFS2_LOCK_BUSY won't be cleared as new master won't
> 	       send ast any more because it thinks already be authorized.
>
> 	spin_lock(&res->spinlock);
> 	lock->convert_pending = 0;
> 	if (status != DLM_NORMAL)
> 		dlm_revert_pending_convert(res, lock);
> 	spin_unlock(&res->spinlock);
> }
>
> In this case, just leave it in convert list and new master will take
> care of it after recovery. And if convert request returns other than
> DLM_NORMAL, convert thread will do the revert itself.
> So remove the revert logic in dlm_move_lockres_to_recovery_list.
>
First, I have a question  about the old code  you are removing.
How did this scenario ever worked?
convert req has been sent to the master and caller of __ocfs2_cluster_lock()
is waiting in ocfs2_wait_for_mask(&mw) when master dies. new master
receives the res with our lock in granted state (since we move it there
  in dlm_move_lockres_to_recovery_list). Now, the new master has no idea
  that this lock needs upconversion. How will it get converted?

Now, back to your fix.This is my understanding.
If we leave it in the converting list as you propose, then
new master will also have it in converting list , its correct place.

dlm_shuffle_lists runs after recovery. Will send BAST to the new master 
which
would behave just like the original master would have. If lock can be 
converted,
it will send AST, otherwise it will wait until it can be, just like the 
old master would
have done etc etc. Am I correct?
Thanks
-Tariq