Changwei Ge
2019-Feb-21 06:46 UTC
[Ocfs2-devel] [PATCH] ocfs2/dlm: return DLM_CANCELGRANT if the lock is on granted list and the operation is canceled
Hi jun Good afternoon.>>>> If AST doesn't manage to get back to requested node, why must flag OCFS2_LOCK_BUSY be cleared in o2dlm_unlock_ast_wrapper? >>>> >>>> Yes, OCFS2_LOCK_BUSY can be cleared it either o2dlm_unlock_ast_wrapper() or o2dlm_lock_ast_wrapper() with o2cb stack applied. >>>> >>>> If we return DLM_CANCELGRANT from ocfs2/dlm to dlm, then we must know that AST has ever come back or master node has moved the lock to grant list itself, thus we clear flag OCFS2_LOCK_BUSY in o2dlm_lock_ast_wrapper(). >>>> Otherwise we ascertain that we can stop current ongoing locking procedure and must clear OCFS2_LOCK_BUSY in o2dlm_unlock_ast_wrapper() (*synchronized*). >>>> >>>> Let's summarize this, OCFS2_LOCK_BUSY should be cleared whether by locking success or cancellation success. >>>> >>>> And my way already check if the lock is granted then return DLM_CANCELGRANT or not. >>>> >>> >>> OCFS2_LOCK_BUSY won't be cleared if DLM_CANCELGRANT is set in >>> o2dlm_unlock_ast_wrapper, and that's what I'm concerned about: >> >> But we already *ascertain* that previous locking request has been *granted* before deciding to return DLM_CANCELGRANT during cancellation to o2dlm_unlock_ast_wrapper(). >> >> If above condition stands, o2dlm_lock_ast_wrapper() must will be or have been called, which also clears OCFS2_LOCK_BUSY. >> > > 1. Node1 already has PR lock, and wants to get ex.Well, a locking up-conversion procedure.> 2. Node1 receive BAST and do unlock, here OCFS2_LOCK_BUSY is set.Because there are two concurrent up-conversion, which conflict, so one of them must be canceled!> 3. Node1 can not receive the AST for unlock as master dead.So here you mean the lock can't be granted.> 4. Then o2dlm_unlock_ast_wrapper will be called rather than o2dlm_lock_ast_wrapper.Then the cancellation succeeds as the master dies.> 5. Actually the *granted* lock request has nothing to do with OCFS2_LOCK_BUSY.Yes, o2dlm_lock_ast_wrapper will not clear OCFS2_LOCK_BUSY. But my suggestion was not against above timing sequence. Did you misunderstand my suggestion? And the original logic of Jian's patch also returns DLM_CANCELGRANT. Thanks, Changwei
piaojun
2019-Feb-22 03:15 UTC
[Ocfs2-devel] [PATCH] ocfs2/dlm: return DLM_CANCELGRANT if the lock is on granted list and the operation is canceled
Hi Changwei, On 2019/2/21 14:46, Changwei Ge wrote:> Hi jun > Good afternoon. > >>>>> If AST doesn't manage to get back to requested node, why must flag OCFS2_LOCK_BUSY be cleared in o2dlm_unlock_ast_wrapper? >>>>> >>>>> Yes, OCFS2_LOCK_BUSY can be cleared it either o2dlm_unlock_ast_wrapper() or o2dlm_lock_ast_wrapper() with o2cb stack applied. >>>>> >>>>> If we return DLM_CANCELGRANT from ocfs2/dlm to dlm, then we must know that AST has ever come back or master node has moved the lock to grant list itself, thus we clear flag OCFS2_LOCK_BUSY in o2dlm_lock_ast_wrapper(). >>>>> Otherwise we ascertain that we can stop current ongoing locking procedure and must clear OCFS2_LOCK_BUSY in o2dlm_unlock_ast_wrapper() (*synchronized*). >>>>> >>>>> Let's summarize this, OCFS2_LOCK_BUSY should be cleared whether by locking success or cancellation success. >>>>> >>>>> And my way already check if the lock is granted then return DLM_CANCELGRANT or not. >>>>> >>>> >>>> OCFS2_LOCK_BUSY won't be cleared if DLM_CANCELGRANT is set in >>>> o2dlm_unlock_ast_wrapper, and that's what I'm concerned about: >>> >>> But we already *ascertain* that previous locking request has been *granted* before deciding to return DLM_CANCELGRANT during cancellation to o2dlm_unlock_ast_wrapper(). >>> >>> If above condition stands, o2dlm_lock_ast_wrapper() must will be or have been called, which also clears OCFS2_LOCK_BUSY. >>> >> >> 1. Node1 already has PR lock, and wants to get ex. > Well, a locking up-conversion procedure. > >> 2. Node1 receive BAST and do unlock, here OCFS2_LOCK_BUSY is set. > Because there are two concurrent up-conversion, which conflict, so one of them must be canceled! > >> 3. Node1 can not receive the AST for unlock as master dead. > So here you mean the lock can't be granted. > >> 4. Then o2dlm_unlock_ast_wrapper will be called rather than o2dlm_lock_ast_wrapper. > Then the cancellation succeeds as the master dies. > >> 5. Actually the *granted* lock request has nothing to do with OCFS2_LOCK_BUSY. > Yes, o2dlm_lock_ast_wrapper will not clear OCFS2_LOCK_BUSY. > > But my suggestion was not against above timing sequence. > Did you misunderstand my suggestion? > And the original logic of Jian's patch also returns DLM_CANCELGRANT.Yes, Jian's last patch can't solve the problem either, and I think we should find another solution for it. I'm considering deleting the check for DLM_CANCELGRANT, and clear OCFS2_LOCK_BUSY in the following process. Thanks, Jun> > Thanks, > Changwei > > > > > . >