* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing
@ 2016-01-11 2:46 xuejiufei
2016-01-12 4:03 ` Junxiao Bi
2016-01-21 7:34 ` Junxiao Bi
0 siblings, 2 replies; 15+ messages in thread
From: xuejiufei @ 2016-01-11 2:46 UTC (permalink / raw)
To: ocfs2-devel
Hi all,
We have found a race between refmap setting and clearing which
will cause the lock resource on master is freed before other nodes
purge it.
Node 1 Node 2(master)
dlm_do_master_request
dlm_master_request_handler
-> dlm_lockres_set_refmap_bit
call dlm_purge_lockres after unlock
dlm_deref_handler
-> find lock resource is in
DLM_LOCK_RES_SETREF_INPROG state,
so dispatch a deref work
dlm_purge_lockres succeed.
dlm_do_master_request
dlm_master_request_handler
-> dlm_lockres_set_refmap_bit
deref work trigger, call
dlm_lockres_clear_refmap_bit
to clear Node 1 from refmap
Now Node 2 can purge the lock resource but the owner of lock resource
on Node 1 is still Node 2 which may trigger BUG if the lock resource
is $RECOVERY or other problems.
We have discussed 2 solutions:
1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG
is set. Node 1 will not retry and master send another message to Node 1
after clearing the refmap. Node 1 can purge the lock resource after the
refmap on master is cleared.
2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG
is set, and Node 1 will retry to deref the lockres.
Does anybody has better ideas?
Thanks,
--Jiufei
^ permalink raw reply [flat|nested] 15+ messages in thread* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-11 2:46 [Ocfs2-devel] ocfs2: A race between refmap setting and clearing xuejiufei @ 2016-01-12 4:03 ` Junxiao Bi 2016-01-12 7:16 ` xuejiufei 2016-01-21 7:34 ` Junxiao Bi 1 sibling, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-12 4:03 UTC (permalink / raw) To: ocfs2-devel Hi Jiufei, On 01/11/2016 10:46 AM, xuejiufei wrote: > Hi all, > We have found a race between refmap setting and clearing which > will cause the lock resource on master is freed before other nodes > purge it. > > Node 1 Node 2(master) > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > call dlm_purge_lockres after unlock > dlm_deref_handler > -> find lock resource is in > DLM_LOCK_RES_SETREF_INPROG state, > so dispatch a deref work > dlm_purge_lockres succeed. > > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > > deref work trigger, call > dlm_lockres_clear_refmap_bit > to clear Node 1 from refmap > > Now Node 2 can purge the lock resource but the owner of lock resource > on Node 1 is still Node 2 which may trigger BUG if the lock resource > is $RECOVERY or other problems. > > We have discussed 2 solutions: > 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG > is set. Node 1 will not retry and master send another message to Node 1 > after clearing the refmap. Node 1 can purge the lock resource after the > refmap on master is cleared. > 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG > is set, and Node 1 will retry to deref the lockres. > > Does anybody has better ideas? > dlm_purge_lockres() will wait to drop ref until DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the master during doing master request. And then this flag was cleared when receiving assert master message, can this fix the issue? Thanks, Junxiao. > Thanks, > --Jiufei > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-12 4:03 ` Junxiao Bi @ 2016-01-12 7:16 ` xuejiufei 2016-01-13 2:46 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: xuejiufei @ 2016-01-12 7:16 UTC (permalink / raw) To: ocfs2-devel Hi, Junxiao On 2016/1/12 12:03, Junxiao Bi wrote: > Hi Jiufei, > > On 01/11/2016 10:46 AM, xuejiufei wrote: >> Hi all, >> We have found a race between refmap setting and clearing which >> will cause the lock resource on master is freed before other nodes >> purge it. >> >> Node 1 Node 2(master) >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> call dlm_purge_lockres after unlock >> dlm_deref_handler >> -> find lock resource is in >> DLM_LOCK_RES_SETREF_INPROG state, >> so dispatch a deref work >> dlm_purge_lockres succeed. >> >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> >> deref work trigger, call >> dlm_lockres_clear_refmap_bit >> to clear Node 1 from refmap >> >> Now Node 2 can purge the lock resource but the owner of lock resource >> on Node 1 is still Node 2 which may trigger BUG if the lock resource >> is $RECOVERY or other problems. >> >> We have discussed 2 solutions: >> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >> is set. Node 1 will not retry and master send another message to Node 1 >> after clearing the refmap. Node 1 can purge the lock resource after the >> refmap on master is cleared. >> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >> is set, and Node 1 will retry to deref the lockres. >> >> Does anybody has better ideas? >> > dlm_purge_lockres() will wait to drop ref until > DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the > master during doing master request. And then this flag was cleared when > receiving assert master message, can this fix the issue? > I don't think this can fix. Before doing master request, the lock resource is already purged. The master should clear the refmap before client purge it. Thanks, Jiufei > Thanks, > Junxiao. >> Thanks, >> --Jiufei >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-12 7:16 ` xuejiufei @ 2016-01-13 2:46 ` Junxiao Bi 2016-01-13 6:21 ` xuejiufei 0 siblings, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-13 2:46 UTC (permalink / raw) To: ocfs2-devel On 01/12/2016 03:16 PM, xuejiufei wrote: > Hi, Junxiao > > On 2016/1/12 12:03, Junxiao Bi wrote: >> Hi Jiufei, >> >> On 01/11/2016 10:46 AM, xuejiufei wrote: >>> Hi all, >>> We have found a race between refmap setting and clearing which >>> will cause the lock resource on master is freed before other nodes >>> purge it. >>> >>> Node 1 Node 2(master) >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> call dlm_purge_lockres after unlock >>> dlm_deref_handler >>> -> find lock resource is in >>> DLM_LOCK_RES_SETREF_INPROG state, >>> so dispatch a deref work >>> dlm_purge_lockres succeed. >>> >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> >>> deref work trigger, call >>> dlm_lockres_clear_refmap_bit >>> to clear Node 1 from refmap >>> >>> Now Node 2 can purge the lock resource but the owner of lock resource >>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>> is $RECOVERY or other problems. >>> >>> We have discussed 2 solutions: >>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>> is set. Node 1 will not retry and master send another message to Node 1 >>> after clearing the refmap. Node 1 can purge the lock resource after the >>> refmap on master is cleared. >>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>> is set, and Node 1 will retry to deref the lockres. >>> >>> Does anybody has better ideas? >>> >> dlm_purge_lockres() will wait to drop ref until >> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >> master during doing master request. And then this flag was cleared when >> receiving assert master message, can this fix the issue? >> > I don't think this can fix. Before doing master request, the lock resource is > already purged. The master should clear the refmap before client purge it. inflight_locks is increased in dlm_get_lock_resource() which will stop lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner during master request, then this will stop lockres purged after unlock? Thanks, Junxiao. > > Thanks, > Jiufei > >> Thanks, >> Junxiao. >>> Thanks, >>> --Jiufei >>> >> >> >> . >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-13 2:46 ` Junxiao Bi @ 2016-01-13 6:21 ` xuejiufei 2016-01-13 7:00 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: xuejiufei @ 2016-01-13 6:21 UTC (permalink / raw) To: ocfs2-devel Hi Junxiao, I have not describe the issue clearly. Node 1 Node 2(master) dlmlock dlm_do_master_request dlm_master_request_handler -> dlm_lockres_set_refmap_bit dlmlock succeed dlmunlock succeed dlm_purge_lockres dlm_deref_handler -> find lock resource is in DLM_LOCK_RES_SETREF_INPROG state, so dispatch a deref work dlm_purge_lockres succeed. call dlmlock again dlm_do_master_request dlm_master_request_handler -> dlm_lockres_set_refmap_bit deref work trigger, call dlm_lockres_clear_refmap_bit to clear Node 1 from refmap dlm_purge_lockres succeed dlm_send_remote_lock_request return DLM_IVLOCKID because the lockres is not exist BUG if the lockres is $RECOVERY On 2016/1/13 10:46, Junxiao Bi wrote: > On 01/12/2016 03:16 PM, xuejiufei wrote: >> Hi, Junxiao >> >> On 2016/1/12 12:03, Junxiao Bi wrote: >>> Hi Jiufei, >>> >>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>> Hi all, >>>> We have found a race between refmap setting and clearing which >>>> will cause the lock resource on master is freed before other nodes >>>> purge it. >>>> >>>> Node 1 Node 2(master) >>>> dlm_do_master_request >>>> dlm_master_request_handler >>>> -> dlm_lockres_set_refmap_bit >>>> call dlm_purge_lockres after unlock >>>> dlm_deref_handler >>>> -> find lock resource is in >>>> DLM_LOCK_RES_SETREF_INPROG state, >>>> so dispatch a deref work >>>> dlm_purge_lockres succeed. >>>> >>>> dlm_do_master_request >>>> dlm_master_request_handler >>>> -> dlm_lockres_set_refmap_bit >>>> >>>> deref work trigger, call >>>> dlm_lockres_clear_refmap_bit >>>> to clear Node 1 from refmap >>>> >>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>> is $RECOVERY or other problems. >>>> >>>> We have discussed 2 solutions: >>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>> is set. Node 1 will not retry and master send another message to Node 1 >>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>> refmap on master is cleared. >>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>> is set, and Node 1 will retry to deref the lockres. >>>> >>>> Does anybody has better ideas? >>>> >>> dlm_purge_lockres() will wait to drop ref until >>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>> master during doing master request. And then this flag was cleared when >>> receiving assert master message, can this fix the issue? >>> >> I don't think this can fix. Before doing master request, the lock resource is >> already purged. The master should clear the refmap before client purge it. > inflight_locks is increased in dlm_get_lock_resource() which will stop > lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner > during master request, then this will stop lockres purged after unlock? > > Thanks, > Junxiao. > >> >> Thanks, >> Jiufei >> >>> Thanks, >>> Junxiao. >>>> Thanks, >>>> --Jiufei >>>> >>> >>> >>> . >>> >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-13 6:21 ` xuejiufei @ 2016-01-13 7:00 ` Junxiao Bi 2016-01-13 8:24 ` Joseph Qi 0 siblings, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-13 7:00 UTC (permalink / raw) To: ocfs2-devel On 01/13/2016 02:21 PM, xuejiufei wrote: > Hi Junxiao, > I have not describe the issue clearly. > > Node 1 Node 2(master) > dlmlock > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > dlmlock succeed > dlmunlock succeed > > dlm_purge_lockres > dlm_deref_handler > -> find lock resource is in > DLM_LOCK_RES_SETREF_INPROG state, > so dispatch a deref work > dlm_purge_lockres succeed. > > call dlmlock again > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > > deref work trigger, call > dlm_lockres_clear_refmap_bit > to clear Node 1 from refmap > > dlm_purge_lockres succeed > > dlm_send_remote_lock_request > return DLM_IVLOCKID because > the lockres is not exist More clear now. Thank you. This is a very complicated race. I didn't have a good solution to fix it now. Your fix looks work, but I am afraid if we keep going fix this kinds of races case by case, we will make dlm harder to understand and easy to involve bugs, maybe we should think about refactor dlm. Thanks, Junxiao. > BUG if the lockres is $RECOVERY > > On 2016/1/13 10:46, Junxiao Bi wrote: >> On 01/12/2016 03:16 PM, xuejiufei wrote: >>> Hi, Junxiao >>> >>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>> Hi Jiufei, >>>> >>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>> Hi all, >>>>> We have found a race between refmap setting and clearing which >>>>> will cause the lock resource on master is freed before other nodes >>>>> purge it. >>>>> >>>>> Node 1 Node 2(master) >>>>> dlm_do_master_request >>>>> dlm_master_request_handler >>>>> -> dlm_lockres_set_refmap_bit >>>>> call dlm_purge_lockres after unlock >>>>> dlm_deref_handler >>>>> -> find lock resource is in >>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>> so dispatch a deref work >>>>> dlm_purge_lockres succeed. >>>>> >>>>> dlm_do_master_request >>>>> dlm_master_request_handler >>>>> -> dlm_lockres_set_refmap_bit >>>>> >>>>> deref work trigger, call >>>>> dlm_lockres_clear_refmap_bit >>>>> to clear Node 1 from refmap >>>>> >>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>> is $RECOVERY or other problems. >>>>> >>>>> We have discussed 2 solutions: >>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>> refmap on master is cleared. >>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>> is set, and Node 1 will retry to deref the lockres. >>>>> >>>>> Does anybody has better ideas? >>>>> >>>> dlm_purge_lockres() will wait to drop ref until >>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>> master during doing master request. And then this flag was cleared when >>>> receiving assert master message, can this fix the issue? >>>> >>> I don't think this can fix. Before doing master request, the lock resource is >>> already purged. The master should clear the refmap before client purge it. >> inflight_locks is increased in dlm_get_lock_resource() which will stop >> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >> during master request, then this will stop lockres purged after unlock? >> >> Thanks, >> Junxiao. >> >>> >>> Thanks, >>> Jiufei >>> >>>> Thanks, >>>> Junxiao. >>>>> Thanks, >>>>> --Jiufei >>>>> >>>> >>>> >>>> . >>>> >>> >> >> >> . >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-13 7:00 ` Junxiao Bi @ 2016-01-13 8:24 ` Joseph Qi 2016-01-18 4:28 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: Joseph Qi @ 2016-01-13 8:24 UTC (permalink / raw) To: ocfs2-devel Hi Junxiao, On 2016/1/13 15:00, Junxiao Bi wrote: > On 01/13/2016 02:21 PM, xuejiufei wrote: >> Hi Junxiao, >> I have not describe the issue clearly. >> >> Node 1 Node 2(master) >> dlmlock >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> dlmlock succeed >> dlmunlock succeed >> >> dlm_purge_lockres >> dlm_deref_handler >> -> find lock resource is in >> DLM_LOCK_RES_SETREF_INPROG state, >> so dispatch a deref work >> dlm_purge_lockres succeed. >> >> call dlmlock again >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> >> deref work trigger, call >> dlm_lockres_clear_refmap_bit >> to clear Node 1 from refmap >> >> dlm_purge_lockres succeed >> >> dlm_send_remote_lock_request >> return DLM_IVLOCKID because >> the lockres is not exist > More clear now. Thank you. > This is a very complicated race. I didn't have a good solution to fix it > now. Your fix looks work, but I am afraid if we keep going fix this > kinds of races case by case, we will make dlm harder to understand and > easy to involve bugs, maybe we should think about refactor dlm. > Agree. IMO, the root cause is bit op cannot handle such a case. I wonder if we have to change it to refcount, which may require a much bigger refactoring. Thanks, Joseph > Thanks, > Junxiao. > >> BUG if the lockres is $RECOVERY >> >> On 2016/1/13 10:46, Junxiao Bi wrote: >>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>> Hi, Junxiao >>>> >>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>> Hi Jiufei, >>>>> >>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>> Hi all, >>>>>> We have found a race between refmap setting and clearing which >>>>>> will cause the lock resource on master is freed before other nodes >>>>>> purge it. >>>>>> >>>>>> Node 1 Node 2(master) >>>>>> dlm_do_master_request >>>>>> dlm_master_request_handler >>>>>> -> dlm_lockres_set_refmap_bit >>>>>> call dlm_purge_lockres after unlock >>>>>> dlm_deref_handler >>>>>> -> find lock resource is in >>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>> so dispatch a deref work >>>>>> dlm_purge_lockres succeed. >>>>>> >>>>>> dlm_do_master_request >>>>>> dlm_master_request_handler >>>>>> -> dlm_lockres_set_refmap_bit >>>>>> >>>>>> deref work trigger, call >>>>>> dlm_lockres_clear_refmap_bit >>>>>> to clear Node 1 from refmap >>>>>> >>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>> is $RECOVERY or other problems. >>>>>> >>>>>> We have discussed 2 solutions: >>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>> refmap on master is cleared. >>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>> >>>>>> Does anybody has better ideas? >>>>>> >>>>> dlm_purge_lockres() will wait to drop ref until >>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>> master during doing master request. And then this flag was cleared when >>>>> receiving assert master message, can this fix the issue? >>>>> >>>> I don't think this can fix. Before doing master request, the lock resource is >>>> already purged. The master should clear the refmap before client purge it. >>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>> during master request, then this will stop lockres purged after unlock? >>> >>> Thanks, >>> Junxiao. >>> >>>> >>>> Thanks, >>>> Jiufei >>>> >>>>> Thanks, >>>>> Junxiao. >>>>>> Thanks, >>>>>> --Jiufei >>>>>> >>>>> >>>>> >>>>> . >>>>> >>>> >>> >>> >>> . >>> >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-13 8:24 ` Joseph Qi @ 2016-01-18 4:28 ` Junxiao Bi 2016-01-18 7:07 ` xuejiufei 0 siblings, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-18 4:28 UTC (permalink / raw) To: ocfs2-devel On 01/13/2016 04:24 PM, Joseph Qi wrote: > Hi Junxiao, > > On 2016/1/13 15:00, Junxiao Bi wrote: >> On 01/13/2016 02:21 PM, xuejiufei wrote: >>> Hi Junxiao, >>> I have not describe the issue clearly. >>> >>> Node 1 Node 2(master) >>> dlmlock >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> dlmlock succeed >>> dlmunlock succeed >>> >>> dlm_purge_lockres >>> dlm_deref_handler >>> -> find lock resource is in >>> DLM_LOCK_RES_SETREF_INPROG state, >>> so dispatch a deref work >>> dlm_purge_lockres succeed. >>> >>> call dlmlock again >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> >>> deref work trigger, call >>> dlm_lockres_clear_refmap_bit >>> to clear Node 1 from refmap >>> >>> dlm_purge_lockres succeed >>> >>> dlm_send_remote_lock_request >>> return DLM_IVLOCKID because >>> the lockres is not exist >> More clear now. Thank you. >> This is a very complicated race. I didn't have a good solution to fix it >> now. Your fix looks work, but I am afraid if we keep going fix this >> kinds of races case by case, we will make dlm harder to understand and >> easy to involve bugs, maybe we should think about refactor dlm. >> > Agree. IMO, the root cause is bit op cannot handle such a case. > I wonder if we have to change it to refcount, which may require a much > bigger refactoring. one bit for each node seems reasonable, as lockres is per node. I think the cause is the dis-order of set/clear, i am trying to see whether they can be made happen in order. Thanks, Junxiao. > > Thanks, > Joseph > >> Thanks, >> Junxiao. >> >>> BUG if the lockres is $RECOVERY >>> >>> On 2016/1/13 10:46, Junxiao Bi wrote: >>>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>>> Hi, Junxiao >>>>> >>>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>>> Hi Jiufei, >>>>>> >>>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>>> Hi all, >>>>>>> We have found a race between refmap setting and clearing which >>>>>>> will cause the lock resource on master is freed before other nodes >>>>>>> purge it. >>>>>>> >>>>>>> Node 1 Node 2(master) >>>>>>> dlm_do_master_request >>>>>>> dlm_master_request_handler >>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>> call dlm_purge_lockres after unlock >>>>>>> dlm_deref_handler >>>>>>> -> find lock resource is in >>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>> so dispatch a deref work >>>>>>> dlm_purge_lockres succeed. >>>>>>> >>>>>>> dlm_do_master_request >>>>>>> dlm_master_request_handler >>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>> >>>>>>> deref work trigger, call >>>>>>> dlm_lockres_clear_refmap_bit >>>>>>> to clear Node 1 from refmap >>>>>>> >>>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>>> is $RECOVERY or other problems. >>>>>>> >>>>>>> We have discussed 2 solutions: >>>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>>> refmap on master is cleared. >>>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>>> >>>>>>> Does anybody has better ideas? >>>>>>> >>>>>> dlm_purge_lockres() will wait to drop ref until >>>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>>> master during doing master request. And then this flag was cleared when >>>>>> receiving assert master message, can this fix the issue? >>>>>> >>>>> I don't think this can fix. Before doing master request, the lock resource is >>>>> already purged. The master should clear the refmap before client purge it. >>>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>>> during master request, then this will stop lockres purged after unlock? >>>> >>>> Thanks, >>>> Junxiao. >>>> >>>>> >>>>> Thanks, >>>>> Jiufei >>>>> >>>>>> Thanks, >>>>>> Junxiao. >>>>>>> Thanks, >>>>>>> --Jiufei >>>>>>> >>>>>> >>>>>> >>>>>> . >>>>>> >>>>> >>>> >>>> >>>> . >>>> >>> >> >> >> . >> > > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-18 4:28 ` Junxiao Bi @ 2016-01-18 7:07 ` xuejiufei 2016-01-19 3:03 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: xuejiufei @ 2016-01-18 7:07 UTC (permalink / raw) To: ocfs2-devel On 2016/1/18 12:28, Junxiao Bi wrote: > On 01/13/2016 04:24 PM, Joseph Qi wrote: >> Hi Junxiao, >> >> On 2016/1/13 15:00, Junxiao Bi wrote: >>> On 01/13/2016 02:21 PM, xuejiufei wrote: >>>> Hi Junxiao, >>>> I have not describe the issue clearly. >>>> >>>> Node 1 Node 2(master) >>>> dlmlock >>>> dlm_do_master_request >>>> dlm_master_request_handler >>>> -> dlm_lockres_set_refmap_bit >>>> dlmlock succeed >>>> dlmunlock succeed >>>> >>>> dlm_purge_lockres >>>> dlm_deref_handler >>>> -> find lock resource is in >>>> DLM_LOCK_RES_SETREF_INPROG state, >>>> so dispatch a deref work >>>> dlm_purge_lockres succeed. >>>> >>>> call dlmlock again >>>> dlm_do_master_request >>>> dlm_master_request_handler >>>> -> dlm_lockres_set_refmap_bit >>>> >>>> deref work trigger, call >>>> dlm_lockres_clear_refmap_bit >>>> to clear Node 1 from refmap >>>> >>>> dlm_purge_lockres succeed >>>> >>>> dlm_send_remote_lock_request >>>> return DLM_IVLOCKID because >>>> the lockres is not exist >>> More clear now. Thank you. >>> This is a very complicated race. I didn't have a good solution to fix it >>> now. Your fix looks work, but I am afraid if we keep going fix this >>> kinds of races case by case, we will make dlm harder to understand and >>> easy to involve bugs, maybe we should think about refactor dlm. >>> >> Agree. IMO, the root cause is bit op cannot handle such a case. >> I wonder if we have to change it to refcount, which may require a much >> bigger refactoring. > one bit for each node seems reasonable, as lockres is per node. I think > the cause is the dis-order of set/clear, i am trying to see whether they > can be made happen in order. > Agree. The solution 1) in my first mail is going to add a new message to keep the order of set and clear. Other nodes can purge the lock resource only after the refmap on master is cleared. Thanks Jiufei > Thanks, > Junxiao. >> >> Thanks, >> Joseph >> >>> Thanks, >>> Junxiao. >>> >>>> BUG if the lockres is $RECOVERY >>>> >>>> On 2016/1/13 10:46, Junxiao Bi wrote: >>>>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>>>> Hi, Junxiao >>>>>> >>>>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>>>> Hi Jiufei, >>>>>>> >>>>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>>>> Hi all, >>>>>>>> We have found a race between refmap setting and clearing which >>>>>>>> will cause the lock resource on master is freed before other nodes >>>>>>>> purge it. >>>>>>>> >>>>>>>> Node 1 Node 2(master) >>>>>>>> dlm_do_master_request >>>>>>>> dlm_master_request_handler >>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>> call dlm_purge_lockres after unlock >>>>>>>> dlm_deref_handler >>>>>>>> -> find lock resource is in >>>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>>> so dispatch a deref work >>>>>>>> dlm_purge_lockres succeed. >>>>>>>> >>>>>>>> dlm_do_master_request >>>>>>>> dlm_master_request_handler >>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>> >>>>>>>> deref work trigger, call >>>>>>>> dlm_lockres_clear_refmap_bit >>>>>>>> to clear Node 1 from refmap >>>>>>>> >>>>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>>>> is $RECOVERY or other problems. >>>>>>>> >>>>>>>> We have discussed 2 solutions: >>>>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>>>> refmap on master is cleared. >>>>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>>>> >>>>>>>> Does anybody has better ideas? >>>>>>>> >>>>>>> dlm_purge_lockres() will wait to drop ref until >>>>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>>>> master during doing master request. And then this flag was cleared when >>>>>>> receiving assert master message, can this fix the issue? >>>>>>> >>>>>> I don't think this can fix. Before doing master request, the lock resource is >>>>>> already purged. The master should clear the refmap before client purge it. >>>>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>>>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>>>> during master request, then this will stop lockres purged after unlock? >>>>> >>>>> Thanks, >>>>> Junxiao. >>>>> >>>>>> >>>>>> Thanks, >>>>>> Jiufei >>>>>> >>>>>>> Thanks, >>>>>>> Junxiao. >>>>>>>> Thanks, >>>>>>>> --Jiufei >>>>>>>> >>>>>>> >>>>>>> >>>>>>> . >>>>>>> >>>>>> >>>>> >>>>> >>>>> . >>>>> >>>> >>> >>> >>> . >>> >> >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-18 7:07 ` xuejiufei @ 2016-01-19 3:03 ` Junxiao Bi 2016-01-19 8:19 ` xuejiufei 0 siblings, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-19 3:03 UTC (permalink / raw) To: ocfs2-devel Hi Jiufei & Joseph, On 01/18/2016 03:07 PM, xuejiufei wrote: > > > On 2016/1/18 12:28, Junxiao Bi wrote: >> On 01/13/2016 04:24 PM, Joseph Qi wrote: >>> Hi Junxiao, >>> >>> On 2016/1/13 15:00, Junxiao Bi wrote: >>>> On 01/13/2016 02:21 PM, xuejiufei wrote: >>>>> Hi Junxiao, >>>>> I have not describe the issue clearly. >>>>> >>>>> Node 1 Node 2(master) >>>>> dlmlock >>>>> dlm_do_master_request >>>>> dlm_master_request_handler >>>>> -> dlm_lockres_set_refmap_bit >>>>> dlmlock succeed >>>>> dlmunlock succeed >>>>> >>>>> dlm_purge_lockres >>>>> dlm_deref_handler >>>>> -> find lock resource is in >>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>> so dispatch a deref work >>>>> dlm_purge_lockres succeed. >>>>> >>>>> call dlmlock again >>>>> dlm_do_master_request >>>>> dlm_master_request_handler >>>>> -> dlm_lockres_set_refmap_bit >>>>> >>>>> deref work trigger, call >>>>> dlm_lockres_clear_refmap_bit >>>>> to clear Node 1 from refmap >>>>> >>>>> dlm_purge_lockres succeed >>>>> >>>>> dlm_send_remote_lock_request >>>>> return DLM_IVLOCKID because >>>>> the lockres is not exist >>>> More clear now. Thank you. >>>> This is a very complicated race. I didn't have a good solution to fix it >>>> now. Your fix looks work, but I am afraid if we keep going fix this >>>> kinds of races case by case, we will make dlm harder to understand and >>>> easy to involve bugs, maybe we should think about refactor dlm. >>>> >>> Agree. IMO, the root cause is bit op cannot handle such a case. >>> I wonder if we have to change it to refcount, which may require a much >>> bigger refactoring. >> one bit for each node seems reasonable, as lockres is per node. I think >> the cause is the dis-order of set/clear, i am trying to see whether they >> can be made happen in order. >> > Agree. The solution 1) in my first mail is going to add a new message to > keep the order of set and clear. Other nodes can purge the lock resource > only after the refmap on master is cleared. I am taking another way, try to delete deref_lockres_worker, and make deref refmap happen directly in deref handler. This needs find the dis-order part and fix. As I can see, the dis-order can only happen at do_assert_master set refmap after deref_lockres_handler clear it? For this, need make sure not return MASTERY_REF to owner after purge. SETREF_INPROG is designed to do this. It is set by assert_master_handler and purge will wait it cleared before deref lockres. There are two cases where SETREF_INPROG is not set when purge 1. assert master message not coming when purge() This happened when lockres owner already exist when asking for a lockres. Lockres owner will be known from master request message and dlm_get_lock_resouces return. Soon dlmlock/dlmunlock is done, and purge done. Then assert master message coming, but since MLE have been deleted in dlm_get_lock_resouce, it will not return MASTERY_REF to owner to reset refmap. So no problem in this case. 2. assert master handler set SETREF_INPROG too late SETREF_INPROG is set after lockres owner has been updated and dlm->spinlock released. That will make dlm_get_lock_resource waken up too fast, and then purge may happen before SETREF_INPROG is set. This is a bug and can be fix by the following patch. diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c index 9477d6e1de37..83cd65b128d0 100644 --- a/fs/ocfs2/dlm/dlmmaster.c +++ b/fs/ocfs2/dlm/dlmmaster.c @@ -1965,6 +1965,7 @@ ok: if (res) { int wake = 0; spin_lock(&res->spinlock); + res->state |= DLM_LOCK_RES_SETREF_INPROG; if (mle->type == DLM_MLE_MIGRATION) { mlog(0, "finishing off migration of lockres %.*s, " "from %u to %u\n", @@ -2029,12 +2030,8 @@ ok: done: ret = 0; - if (res) { - spin_lock(&res->spinlock); - res->state |= DLM_LOCK_RES_SETREF_INPROG; - spin_unlock(&res->spinlock); + if (res) *ret_data = (void *)res; - } dlm_put(dlm); if (master_request) { mlog(0, "need to tell master to reassert\n"); Besides this path, revert commit f3f854648de64c4b6f13f6f13113bc9525c621e5 ("ocfs2_dlm: Ensure correct ordering of set/clear refmap bit on lockres"), can this fix this issue? Thanks, Junxiao. > > Thanks > Jiufei > >> Thanks, >> Junxiao. >>> >>> Thanks, >>> Joseph >>> >>>> Thanks, >>>> Junxiao. >>>> >>>>> BUG if the lockres is $RECOVERY >>>>> >>>>> On 2016/1/13 10:46, Junxiao Bi wrote: >>>>>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>>>>> Hi, Junxiao >>>>>>> >>>>>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>>>>> Hi Jiufei, >>>>>>>> >>>>>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>>>>> Hi all, >>>>>>>>> We have found a race between refmap setting and clearing which >>>>>>>>> will cause the lock resource on master is freed before other nodes >>>>>>>>> purge it. >>>>>>>>> >>>>>>>>> Node 1 Node 2(master) >>>>>>>>> dlm_do_master_request >>>>>>>>> dlm_master_request_handler >>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>> call dlm_purge_lockres after unlock >>>>>>>>> dlm_deref_handler >>>>>>>>> -> find lock resource is in >>>>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>>>> so dispatch a deref work >>>>>>>>> dlm_purge_lockres succeed. >>>>>>>>> >>>>>>>>> dlm_do_master_request >>>>>>>>> dlm_master_request_handler >>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>> >>>>>>>>> deref work trigger, call >>>>>>>>> dlm_lockres_clear_refmap_bit >>>>>>>>> to clear Node 1 from refmap >>>>>>>>> >>>>>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>>>>> is $RECOVERY or other problems. >>>>>>>>> >>>>>>>>> We have discussed 2 solutions: >>>>>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>>>>> refmap on master is cleared. >>>>>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>>>>> >>>>>>>>> Does anybody has better ideas? >>>>>>>>> >>>>>>>> dlm_purge_lockres() will wait to drop ref until >>>>>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>>>>> master during doing master request. And then this flag was cleared when >>>>>>>> receiving assert master message, can this fix the issue? >>>>>>>> >>>>>>> I don't think this can fix. Before doing master request, the lock resource is >>>>>>> already purged. The master should clear the refmap before client purge it. >>>>>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>>>>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>>>>> during master request, then this will stop lockres purged after unlock? >>>>>> >>>>>> Thanks, >>>>>> Junxiao. >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Jiufei >>>>>>> >>>>>>>> Thanks, >>>>>>>> Junxiao. >>>>>>>>> Thanks, >>>>>>>>> --Jiufei >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> . >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> . >>>>>> >>>>> >>>> >>>> >>>> . >>>> >>> >>> >> >> >> . >> > ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-19 3:03 ` Junxiao Bi @ 2016-01-19 8:19 ` xuejiufei 2016-01-19 9:02 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: xuejiufei @ 2016-01-19 8:19 UTC (permalink / raw) To: ocfs2-devel Hi Junxiao, commit f3f854648de64c4b6f13f6f13113bc9525c621e5 is to solve the disorder problem of set/clear refmap on master. If it is reverted, how to fix the problem the patch described? I think the following is the issue that patch fixed. The set refmap bit message is sent before the clear refmap message. But these two message is processed by different processes, so the set refmap message may be handled before the clear. Node 1 Node 2(master) dlm_wq o2net_wq dlm_do_assert_master dlm_do_assert_handler return MASTERY_REF receive MASTERY_REF dlm_purge_lockres send deref message receive deref message -> dlm_deref_lockres_handler and clear refmap -> set refmap On 2016/1/19 11:03, Junxiao Bi wrote: > Hi Jiufei & Joseph, > > On 01/18/2016 03:07 PM, xuejiufei wrote: >> >> >> On 2016/1/18 12:28, Junxiao Bi wrote: >>> On 01/13/2016 04:24 PM, Joseph Qi wrote: >>>> Hi Junxiao, >>>> >>>> On 2016/1/13 15:00, Junxiao Bi wrote: >>>>> On 01/13/2016 02:21 PM, xuejiufei wrote: >>>>>> Hi Junxiao, >>>>>> I have not describe the issue clearly. >>>>>> >>>>>> Node 1 Node 2(master) >>>>>> dlmlock >>>>>> dlm_do_master_request >>>>>> dlm_master_request_handler >>>>>> -> dlm_lockres_set_refmap_bit >>>>>> dlmlock succeed >>>>>> dlmunlock succeed >>>>>> >>>>>> dlm_purge_lockres >>>>>> dlm_deref_handler >>>>>> -> find lock resource is in >>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>> so dispatch a deref work >>>>>> dlm_purge_lockres succeed. >>>>>> >>>>>> call dlmlock again >>>>>> dlm_do_master_request >>>>>> dlm_master_request_handler >>>>>> -> dlm_lockres_set_refmap_bit >>>>>> >>>>>> deref work trigger, call >>>>>> dlm_lockres_clear_refmap_bit >>>>>> to clear Node 1 from refmap >>>>>> >>>>>> dlm_purge_lockres succeed >>>>>> >>>>>> dlm_send_remote_lock_request >>>>>> return DLM_IVLOCKID because >>>>>> the lockres is not exist >>>>> More clear now. Thank you. >>>>> This is a very complicated race. I didn't have a good solution to fix it >>>>> now. Your fix looks work, but I am afraid if we keep going fix this >>>>> kinds of races case by case, we will make dlm harder to understand and >>>>> easy to involve bugs, maybe we should think about refactor dlm. >>>>> >>>> Agree. IMO, the root cause is bit op cannot handle such a case. >>>> I wonder if we have to change it to refcount, which may require a much >>>> bigger refactoring. >>> one bit for each node seems reasonable, as lockres is per node. I think >>> the cause is the dis-order of set/clear, i am trying to see whether they >>> can be made happen in order. >>> >> Agree. The solution 1) in my first mail is going to add a new message to >> keep the order of set and clear. Other nodes can purge the lock resource >> only after the refmap on master is cleared. > I am taking another way, try to delete deref_lockres_worker, and make > deref refmap happen directly in deref handler. This needs find the > dis-order part and fix. As I can see, the dis-order can only happen at > do_assert_master set refmap after deref_lockres_handler clear it? > > For this, need make sure not return MASTERY_REF to owner after purge. > SETREF_INPROG is designed to do this. It is set by assert_master_handler > and purge will wait it cleared before deref lockres. There are two cases > where SETREF_INPROG is not set when purge > > 1. assert master message not coming when purge() > This happened when lockres owner already exist when asking for a > lockres. Lockres owner will be known from master request message and > dlm_get_lock_resouces return. Soon dlmlock/dlmunlock is done, and purge > done. Then assert master message coming, but since MLE have been deleted > in dlm_get_lock_resouce, it will not return MASTERY_REF to owner to > reset refmap. So no problem in this case. > > 2. assert master handler set SETREF_INPROG too late > SETREF_INPROG is set after lockres owner has been updated and > dlm->spinlock released. That will make dlm_get_lock_resource waken up > too fast, and then purge may happen before SETREF_INPROG is set. This is > a bug and can be fix by the following patch. > > diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c > index 9477d6e1de37..83cd65b128d0 100644 > --- a/fs/ocfs2/dlm/dlmmaster.c > +++ b/fs/ocfs2/dlm/dlmmaster.c > @@ -1965,6 +1965,7 @@ ok: > if (res) { > int wake = 0; > spin_lock(&res->spinlock); > + res->state |= DLM_LOCK_RES_SETREF_INPROG; > if (mle->type == DLM_MLE_MIGRATION) { > mlog(0, "finishing off migration of > lockres %.*s, " > "from %u to %u\n", > @@ -2029,12 +2030,8 @@ ok: > > done: > ret = 0; > - if (res) { > - spin_lock(&res->spinlock); > - res->state |= DLM_LOCK_RES_SETREF_INPROG; > - spin_unlock(&res->spinlock); > + if (res) > *ret_data = (void *)res; > - } > dlm_put(dlm); > if (master_request) { > mlog(0, "need to tell master to reassert\n"); > > > Besides this path, revert commit > f3f854648de64c4b6f13f6f13113bc9525c621e5 ("ocfs2_dlm: Ensure correct > ordering of set/clear refmap bit on lockres"), can this fix this issue? > > Thanks, > Junxiao. > > > >> >> Thanks >> Jiufei >> >>> Thanks, >>> Junxiao. >>>> >>>> Thanks, >>>> Joseph >>>> >>>>> Thanks, >>>>> Junxiao. >>>>> >>>>>> BUG if the lockres is $RECOVERY >>>>>> >>>>>> On 2016/1/13 10:46, Junxiao Bi wrote: >>>>>>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>>>>>> Hi, Junxiao >>>>>>>> >>>>>>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>>>>>> Hi Jiufei, >>>>>>>>> >>>>>>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>>>>>> Hi all, >>>>>>>>>> We have found a race between refmap setting and clearing which >>>>>>>>>> will cause the lock resource on master is freed before other nodes >>>>>>>>>> purge it. >>>>>>>>>> >>>>>>>>>> Node 1 Node 2(master) >>>>>>>>>> dlm_do_master_request >>>>>>>>>> dlm_master_request_handler >>>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>>> call dlm_purge_lockres after unlock >>>>>>>>>> dlm_deref_handler >>>>>>>>>> -> find lock resource is in >>>>>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>>>>> so dispatch a deref work >>>>>>>>>> dlm_purge_lockres succeed. >>>>>>>>>> >>>>>>>>>> dlm_do_master_request >>>>>>>>>> dlm_master_request_handler >>>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>>> >>>>>>>>>> deref work trigger, call >>>>>>>>>> dlm_lockres_clear_refmap_bit >>>>>>>>>> to clear Node 1 from refmap >>>>>>>>>> >>>>>>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>>>>>> is $RECOVERY or other problems. >>>>>>>>>> >>>>>>>>>> We have discussed 2 solutions: >>>>>>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>>>>>> refmap on master is cleared. >>>>>>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>>>>>> >>>>>>>>>> Does anybody has better ideas? >>>>>>>>>> >>>>>>>>> dlm_purge_lockres() will wait to drop ref until >>>>>>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>>>>>> master during doing master request. And then this flag was cleared when >>>>>>>>> receiving assert master message, can this fix the issue? >>>>>>>>> >>>>>>>> I don't think this can fix. Before doing master request, the lock resource is >>>>>>>> already purged. The master should clear the refmap before client purge it. >>>>>>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>>>>>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>>>>>> during master request, then this will stop lockres purged after unlock? >>>>>>> >>>>>>> Thanks, >>>>>>> Junxiao. >>>>>>> >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Jiufei >>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Junxiao. >>>>>>>>>> Thanks, >>>>>>>>>> --Jiufei >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> . >>>>>>>>> >>>>>>>> >>>>>>> >>>>>>> >>>>>>> . >>>>>>> >>>>>> >>>>> >>>>> >>>>> . >>>>> >>>> >>>> >>> >>> >>> . >>> >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-19 8:19 ` xuejiufei @ 2016-01-19 9:02 ` Junxiao Bi 0 siblings, 0 replies; 15+ messages in thread From: Junxiao Bi @ 2016-01-19 9:02 UTC (permalink / raw) To: ocfs2-devel Hi Jiufei, Right. Revert that patch will introduce that dis-order, thank you for pointing it out. I will think about more about it. My idea is to do some refactor to make it more smooth while not introducing complicated things to just fix that bug. Thanks, Junxiao. On 01/19/2016 04:19 PM, xuejiufei wrote: > Hi Junxiao, > commit f3f854648de64c4b6f13f6f13113bc9525c621e5 is to solve > the disorder problem of set/clear refmap on master. > If it is reverted, how to fix the problem the patch > described? > > I think the following is the issue that patch fixed. > The set refmap bit message is sent before the clear refmap > message. But these two message is processed by different > processes, so the set refmap message may be handled before the clear. > > Node 1 Node 2(master) > dlm_wq o2net_wq > dlm_do_assert_master > dlm_do_assert_handler > return MASTERY_REF > receive MASTERY_REF > dlm_purge_lockres > send deref message > receive deref message > -> dlm_deref_lockres_handler > and clear refmap > -> set refmap > > On 2016/1/19 11:03, Junxiao Bi wrote: >> Hi Jiufei & Joseph, >> >> On 01/18/2016 03:07 PM, xuejiufei wrote: >>> >>> >>> On 2016/1/18 12:28, Junxiao Bi wrote: >>>> On 01/13/2016 04:24 PM, Joseph Qi wrote: >>>>> Hi Junxiao, >>>>> >>>>> On 2016/1/13 15:00, Junxiao Bi wrote: >>>>>> On 01/13/2016 02:21 PM, xuejiufei wrote: >>>>>>> Hi Junxiao, >>>>>>> I have not describe the issue clearly. >>>>>>> >>>>>>> Node 1 Node 2(master) >>>>>>> dlmlock >>>>>>> dlm_do_master_request >>>>>>> dlm_master_request_handler >>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>> dlmlock succeed >>>>>>> dlmunlock succeed >>>>>>> >>>>>>> dlm_purge_lockres >>>>>>> dlm_deref_handler >>>>>>> -> find lock resource is in >>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>> so dispatch a deref work >>>>>>> dlm_purge_lockres succeed. >>>>>>> >>>>>>> call dlmlock again >>>>>>> dlm_do_master_request >>>>>>> dlm_master_request_handler >>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>> >>>>>>> deref work trigger, call >>>>>>> dlm_lockres_clear_refmap_bit >>>>>>> to clear Node 1 from refmap >>>>>>> >>>>>>> dlm_purge_lockres succeed >>>>>>> >>>>>>> dlm_send_remote_lock_request >>>>>>> return DLM_IVLOCKID because >>>>>>> the lockres is not exist >>>>>> More clear now. Thank you. >>>>>> This is a very complicated race. I didn't have a good solution to fix it >>>>>> now. Your fix looks work, but I am afraid if we keep going fix this >>>>>> kinds of races case by case, we will make dlm harder to understand and >>>>>> easy to involve bugs, maybe we should think about refactor dlm. >>>>>> >>>>> Agree. IMO, the root cause is bit op cannot handle such a case. >>>>> I wonder if we have to change it to refcount, which may require a much >>>>> bigger refactoring. >>>> one bit for each node seems reasonable, as lockres is per node. I think >>>> the cause is the dis-order of set/clear, i am trying to see whether they >>>> can be made happen in order. >>>> >>> Agree. The solution 1) in my first mail is going to add a new message to >>> keep the order of set and clear. Other nodes can purge the lock resource >>> only after the refmap on master is cleared. >> I am taking another way, try to delete deref_lockres_worker, and make >> deref refmap happen directly in deref handler. This needs find the >> dis-order part and fix. As I can see, the dis-order can only happen at >> do_assert_master set refmap after deref_lockres_handler clear it? >> >> For this, need make sure not return MASTERY_REF to owner after purge. >> SETREF_INPROG is designed to do this. It is set by assert_master_handler >> and purge will wait it cleared before deref lockres. There are two cases >> where SETREF_INPROG is not set when purge >> >> 1. assert master message not coming when purge() >> This happened when lockres owner already exist when asking for a >> lockres. Lockres owner will be known from master request message and >> dlm_get_lock_resouces return. Soon dlmlock/dlmunlock is done, and purge >> done. Then assert master message coming, but since MLE have been deleted >> in dlm_get_lock_resouce, it will not return MASTERY_REF to owner to >> reset refmap. So no problem in this case. >> >> 2. assert master handler set SETREF_INPROG too late >> SETREF_INPROG is set after lockres owner has been updated and >> dlm->spinlock released. That will make dlm_get_lock_resource waken up >> too fast, and then purge may happen before SETREF_INPROG is set. This is >> a bug and can be fix by the following patch. >> >> diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c >> index 9477d6e1de37..83cd65b128d0 100644 >> --- a/fs/ocfs2/dlm/dlmmaster.c >> +++ b/fs/ocfs2/dlm/dlmmaster.c >> @@ -1965,6 +1965,7 @@ ok: >> if (res) { >> int wake = 0; >> spin_lock(&res->spinlock); >> + res->state |= DLM_LOCK_RES_SETREF_INPROG; >> if (mle->type == DLM_MLE_MIGRATION) { >> mlog(0, "finishing off migration of >> lockres %.*s, " >> "from %u to %u\n", >> @@ -2029,12 +2030,8 @@ ok: >> >> done: >> ret = 0; >> - if (res) { >> - spin_lock(&res->spinlock); >> - res->state |= DLM_LOCK_RES_SETREF_INPROG; >> - spin_unlock(&res->spinlock); >> + if (res) >> *ret_data = (void *)res; >> - } >> dlm_put(dlm); >> if (master_request) { >> mlog(0, "need to tell master to reassert\n"); >> >> >> Besides this path, revert commit >> f3f854648de64c4b6f13f6f13113bc9525c621e5 ("ocfs2_dlm: Ensure correct >> ordering of set/clear refmap bit on lockres"), can this fix this issue? >> >> Thanks, >> Junxiao. >> >> >> >>> >>> Thanks >>> Jiufei >>> >>>> Thanks, >>>> Junxiao. >>>>> >>>>> Thanks, >>>>> Joseph >>>>> >>>>>> Thanks, >>>>>> Junxiao. >>>>>> >>>>>>> BUG if the lockres is $RECOVERY >>>>>>> >>>>>>> On 2016/1/13 10:46, Junxiao Bi wrote: >>>>>>>> On 01/12/2016 03:16 PM, xuejiufei wrote: >>>>>>>>> Hi, Junxiao >>>>>>>>> >>>>>>>>> On 2016/1/12 12:03, Junxiao Bi wrote: >>>>>>>>>> Hi Jiufei, >>>>>>>>>> >>>>>>>>>> On 01/11/2016 10:46 AM, xuejiufei wrote: >>>>>>>>>>> Hi all, >>>>>>>>>>> We have found a race between refmap setting and clearing which >>>>>>>>>>> will cause the lock resource on master is freed before other nodes >>>>>>>>>>> purge it. >>>>>>>>>>> >>>>>>>>>>> Node 1 Node 2(master) >>>>>>>>>>> dlm_do_master_request >>>>>>>>>>> dlm_master_request_handler >>>>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>>>> call dlm_purge_lockres after unlock >>>>>>>>>>> dlm_deref_handler >>>>>>>>>>> -> find lock resource is in >>>>>>>>>>> DLM_LOCK_RES_SETREF_INPROG state, >>>>>>>>>>> so dispatch a deref work >>>>>>>>>>> dlm_purge_lockres succeed. >>>>>>>>>>> >>>>>>>>>>> dlm_do_master_request >>>>>>>>>>> dlm_master_request_handler >>>>>>>>>>> -> dlm_lockres_set_refmap_bit >>>>>>>>>>> >>>>>>>>>>> deref work trigger, call >>>>>>>>>>> dlm_lockres_clear_refmap_bit >>>>>>>>>>> to clear Node 1 from refmap >>>>>>>>>>> >>>>>>>>>>> Now Node 2 can purge the lock resource but the owner of lock resource >>>>>>>>>>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>>>>>>>>>> is $RECOVERY or other problems. >>>>>>>>>>> >>>>>>>>>>> We have discussed 2 solutions: >>>>>>>>>>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>>>> is set. Node 1 will not retry and master send another message to Node 1 >>>>>>>>>>> after clearing the refmap. Node 1 can purge the lock resource after the >>>>>>>>>>> refmap on master is cleared. >>>>>>>>>>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>>>>>>>>>> is set, and Node 1 will retry to deref the lockres. >>>>>>>>>>> >>>>>>>>>>> Does anybody has better ideas? >>>>>>>>>>> >>>>>>>>>> dlm_purge_lockres() will wait to drop ref until >>>>>>>>>> DLM_LOCK_RES_SETREF_INPROG cleared. So if set this flag when find the >>>>>>>>>> master during doing master request. And then this flag was cleared when >>>>>>>>>> receiving assert master message, can this fix the issue? >>>>>>>>>> >>>>>>>>> I don't think this can fix. Before doing master request, the lock resource is >>>>>>>>> already purged. The master should clear the refmap before client purge it. >>>>>>>> inflight_locks is increased in dlm_get_lock_resource() which will stop >>>>>>>> lockres purged? Set DLM_LOCK_RES_SETREF_INPROG when found lockres owner >>>>>>>> during master request, then this will stop lockres purged after unlock? >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Junxiao. >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Jiufei >>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Junxiao. >>>>>>>>>>> Thanks, >>>>>>>>>>> --Jiufei >>>>>>>>>>> >>>>>>>>>> >>>>>>>>>> >>>>>>>>>> . >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>>> >>>>>>>> . >>>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> . >>>>>> >>>>> >>>>> >>>> >>>> >>>> . >>>> >>> >> >> >> . >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-11 2:46 [Ocfs2-devel] ocfs2: A race between refmap setting and clearing xuejiufei 2016-01-12 4:03 ` Junxiao Bi @ 2016-01-21 7:34 ` Junxiao Bi 2016-01-26 1:43 ` xuejiufei 1 sibling, 1 reply; 15+ messages in thread From: Junxiao Bi @ 2016-01-21 7:34 UTC (permalink / raw) To: ocfs2-devel Hi Jiufei, I didn't find other solution for this issue. You can go with yours. Looks like your second one is more straightforward, there deref work can be removed. Thanks, Junxiao. On 01/11/2016 10:46 AM, xuejiufei wrote: > Hi all, > We have found a race between refmap setting and clearing which > will cause the lock resource on master is freed before other nodes > purge it. > > Node 1 Node 2(master) > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > call dlm_purge_lockres after unlock > dlm_deref_handler > -> find lock resource is in > DLM_LOCK_RES_SETREF_INPROG state, > so dispatch a deref work > dlm_purge_lockres succeed. > > dlm_do_master_request > dlm_master_request_handler > -> dlm_lockres_set_refmap_bit > > deref work trigger, call > dlm_lockres_clear_refmap_bit > to clear Node 1 from refmap > > Now Node 2 can purge the lock resource but the owner of lock resource > on Node 1 is still Node 2 which may trigger BUG if the lock resource > is $RECOVERY or other problems. > > We have discussed 2 solutions: > 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG > is set. Node 1 will not retry and master send another message to Node 1 > after clearing the refmap. Node 1 can purge the lock resource after the > refmap on master is cleared. > 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG > is set, and Node 1 will retry to deref the lockres. > > Does anybody has better ideas? > > Thanks, > --Jiufei > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-21 7:34 ` Junxiao Bi @ 2016-01-26 1:43 ` xuejiufei 2016-01-26 2:45 ` Junxiao Bi 0 siblings, 1 reply; 15+ messages in thread From: xuejiufei @ 2016-01-26 1:43 UTC (permalink / raw) To: ocfs2-devel Hi Junxiao, On 2016/1/21 15:34, Junxiao Bi wrote: > Hi Jiufei, > > I didn't find other solution for this issue. You can go with yours. > Looks like your second one is more straightforward, there deref work can > be removed. > There are two problems with the second solution: 1) Node retry to deref the lock resource will block dlm_thread to process other lock resources. 2) When node retries to drop the refmap bit, the master may be in another assert master work, that will take a long time to purge a lockres. So I prefer the first solution. Thanks, Jiufei > Thanks, > Junxiao. > On 01/11/2016 10:46 AM, xuejiufei wrote: >> Hi all, >> We have found a race between refmap setting and clearing which >> will cause the lock resource on master is freed before other nodes >> purge it. >> >> Node 1 Node 2(master) >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> call dlm_purge_lockres after unlock >> dlm_deref_handler >> -> find lock resource is in >> DLM_LOCK_RES_SETREF_INPROG state, >> so dispatch a deref work >> dlm_purge_lockres succeed. >> >> dlm_do_master_request >> dlm_master_request_handler >> -> dlm_lockres_set_refmap_bit >> >> deref work trigger, call >> dlm_lockres_clear_refmap_bit >> to clear Node 1 from refmap >> >> Now Node 2 can purge the lock resource but the owner of lock resource >> on Node 1 is still Node 2 which may trigger BUG if the lock resource >> is $RECOVERY or other problems. >> >> We have discussed 2 solutions: >> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >> is set. Node 1 will not retry and master send another message to Node 1 >> after clearing the refmap. Node 1 can purge the lock resource after the >> refmap on master is cleared. >> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >> is set, and Node 1 will retry to deref the lockres. >> >> Does anybody has better ideas? >> >> Thanks, >> --Jiufei >> > > > . > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Ocfs2-devel] ocfs2: A race between refmap setting and clearing 2016-01-26 1:43 ` xuejiufei @ 2016-01-26 2:45 ` Junxiao Bi 0 siblings, 0 replies; 15+ messages in thread From: Junxiao Bi @ 2016-01-26 2:45 UTC (permalink / raw) To: ocfs2-devel On 01/26/2016 09:43 AM, xuejiufei wrote: > Hi Junxiao, > > On 2016/1/21 15:34, Junxiao Bi wrote: >> Hi Jiufei, >> >> I didn't find other solution for this issue. You can go with yours. >> Looks like your second one is more straightforward, there deref work can >> be removed. >> > There are two problems with the second solution: > 1) Node retry to deref the lock resource will block dlm_thread to process > other lock resources. Yes, a little, but i don't think that will be long. Indeed I think about clear DLM_LOCK_RES_DROPPING_REF and requeue lockres again before, that will not block dlm_thread, but I found dlm had an assuming about this flag, it assumed the lockres is gone if it is set. If this bad assuming can be fixed, the second solution will be much better. > 2) When node retries to drop the refmap bit, the master may be in another > assert master work, that will take a long time to purge a lockres. Delay purging lockres maybe not a bad idea, but a good one. Like in your case, the second lock request can go directly without master reqeust. This can improve performance. Thanks, Junxiao. > So I prefer the first solution. > > Thanks, > Jiufei > >> Thanks, >> Junxiao. >> On 01/11/2016 10:46 AM, xuejiufei wrote: >>> Hi all, >>> We have found a race between refmap setting and clearing which >>> will cause the lock resource on master is freed before other nodes >>> purge it. >>> >>> Node 1 Node 2(master) >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> call dlm_purge_lockres after unlock >>> dlm_deref_handler >>> -> find lock resource is in >>> DLM_LOCK_RES_SETREF_INPROG state, >>> so dispatch a deref work >>> dlm_purge_lockres succeed. >>> >>> dlm_do_master_request >>> dlm_master_request_handler >>> -> dlm_lockres_set_refmap_bit >>> >>> deref work trigger, call >>> dlm_lockres_clear_refmap_bit >>> to clear Node 1 from refmap >>> >>> Now Node 2 can purge the lock resource but the owner of lock resource >>> on Node 1 is still Node 2 which may trigger BUG if the lock resource >>> is $RECOVERY or other problems. >>> >>> We have discussed 2 solutions: >>> 1)The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>> is set. Node 1 will not retry and master send another message to Node 1 >>> after clearing the refmap. Node 1 can purge the lock resource after the >>> refmap on master is cleared. >>> 2) The master return error to Node 1 if the DLM_LOCK_RES_SETREF_INPROG >>> is set, and Node 1 will retry to deref the lockres. >>> >>> Does anybody has better ideas? >>> >>> Thanks, >>> --Jiufei >>> >> >> >> . >> > ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-01-26 2:45 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-01-11 2:46 [Ocfs2-devel] ocfs2: A race between refmap setting and clearing xuejiufei 2016-01-12 4:03 ` Junxiao Bi 2016-01-12 7:16 ` xuejiufei 2016-01-13 2:46 ` Junxiao Bi 2016-01-13 6:21 ` xuejiufei 2016-01-13 7:00 ` Junxiao Bi 2016-01-13 8:24 ` Joseph Qi 2016-01-18 4:28 ` Junxiao Bi 2016-01-18 7:07 ` xuejiufei 2016-01-19 3:03 ` Junxiao Bi 2016-01-19 8:19 ` xuejiufei 2016-01-19 9:02 ` Junxiao Bi 2016-01-21 7:34 ` Junxiao Bi 2016-01-26 1:43 ` xuejiufei 2016-01-26 2:45 ` Junxiao Bi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).