ocfs2/dlm: ignore cleaning the migration mle that is inuse
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2. dlm_send_one_lockres and wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockres woken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".
--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)
fs/ocfs2/dlm/dlmmaster.c +15 -11
@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);
@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;
@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a
@@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { +mlog (ML_NOTICE ,"%s: target %u died during " +"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);
Signature · 1 line
1.8.4.3
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()? mle->woken is set to 1 in dlm_clean_migration_mle(), so the code to detect target node down(if (dlm_is_node_dead(dlm, target))) will never be run in dlm_migrate_lockres()?
2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624 msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken) == 1 ||
2628 res->owner == target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632 dlm->name, res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638 dlm->name, res->lockname.len,
2639 res->lockname.name, target);
2640 ret = -EINVAL;
2641 /* migration failed, detach and
clean up mle */
2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;
2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653 dlm->name, res->lockname.len,res->lockname.name);
2654 }
Thanks,
Junxiao.
Quoted 84 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +15
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { +mlog (ML_NOTICE ,"%s: target %u died during " +"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Hi Junxiao,
You are right. But it may happen that mle->woken is set to 1 in dlm_clean_migration_mle() just after atomic_read() in dlm_migrate_lockres(). Actually we trigger this BUG when dlm_send_one_lockres() return error.
And I think dlm_migrate_lockres() should not set owner to target and return 0 when mle->woken is set to 1 in dlm_clean_migration_mle(). This is another problem?
Thanks
Jiufei.
Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Quoted 5 lines · xuejiufei
Hi Junxiao,
You are right. But it may happen thatmle->wokenis set to 1 indlm_clean_migration_mle()just afteratomic_read()indlm_migrate_lockres(). Actually we trigger this BUG whendlm_send_one_lockres()return error.
Yes, that's possible because of that 5s timeout wakeup, I think this timeout is useless and can be removed?
Quoted 3 lines
And I think
dlm_migrate_lockres()should not set owner to target and return 0 whenmle->wokenis set to 1 indlm_clean_migration_mle(). This is another problem?
Yes, it should be fixed, or the lockres owner will be set to a down node wrongly. Can the following fix these two issue?
fs/ocfs2/dlm/dlmmaster.c +16 -33
@@ -2618,39 +2618,22 @@ fail:/* wait for new node to assert master */ -while (1 ) { - ret =wait_event_interruptible_timeout (mle->wq, - (atomic_read (&mle->woken) ==1 ), -msecs_to_jiffies (5000 )); - -if (ret >=0 ) { -if (atomic_read (&mle->woken) ==1 || - res->owner == target) -break ; - -mlog (0 ,"%s:%.*s: timed out during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); -/* avoid hang during shutdown when migrating lockres -* to a node which also goes down */ -if (dlm_is_node_dead (dlm, target)) { -mlog (0 ,"%s:%.*s: expected migration " -"target %u is no longer up, restarting\n" , - dlm->name, res->lockname.len, - res->lockname.name, target); - ret = -EINVAL ; -/* migration failed, detach and clean up mle */ -dlm_mle_detach_hb_events (dlm, mle); -dlm_put_mle (mle); -dlm_put_mle_inuse (mle); -spin_lock (&res->spinlock); - res->state &= ~DLM_LOCK_RES_MIGRATING ; - wake =1 ; -spin_unlock (&res->spinlock); - goto leave; - } - }else -mlog (0 ,"%s:%.*s: caught signal during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); +wait_event (mle->wq, ((atomic_read (&mle->woken) ==1 ) || + (atomic_read (&mle->woken) ==2 ))); + +/* migrate target down */ +if (atomic_read (&mle->woken) ==2 ) { +mlog (0 ,"%s:%.*s: expected migration " +"target %u is no longer up, restarting\n" , + dlm->name, res->lockname.len, + res->lockname.name, target); + ret = -EINVAL ; +dlm_put_mle_inuse (mle); +spin_lock (&res->spinlock); + res->state &= ~DLM_LOCK_RES_MIGRATING ; + wake =1 ; +spin_unlock (&res->spinlock); + goto leave; }/* all done, set the owner, clear the flag */
@@ -3227,7 +3210,7 @@ static void dlm_clean_migration_mle(struct
dlm_ctxt *dlm,
spin_lock(&mle->spinlock);
__dlm_unlink_mle(dlm, mle);
- atomic_set(&mle->woken, 1);
+ atomic_set(&mle->woken, 2);
spin_unlock(&mle->spinlock);
wake_up(&mle->wq);
Thanks,
Junxiao.
Quoted 150 lines
Thanks
Jiufei.Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Hi Junxiao,
Quoted 6 lines · Junxiao Bi
Quoted 5 lines · xuejiufei
Hi Junxiao,
You are right. But it may happen thatmle->wokenis set to 1 indlm_clean_migration_mle()just afteratomic_read()indlm_migrate_lockres(). Actually we trigger this BUG whendlm_send_one_lockres()return error.
Quoted 8 lines
Yes, that's possible because of that 5s timeout wakeup, I think this timeout is useless and can be removed?
Quoted 3 lines
And I think
dlm_migrate_lockres()should not set owner to target and return 0 whenmle->wokenis set to 1 indlm_clean_migration_mle(). This is another problem?Yes, it should be fixed, or the lockres owner will be set to a down node wrongly. Can the following fix these two issue?
It can not fix the first issue when dlm_send_one_lockres() or dlm_mark_lockres_migrating() return error, right?
Quoted 230 lines
fs/ocfs2/dlm/dlmmaster.c +17
-34@@ -2618,39 +2618,22 @@ fail:/* wait for new node to assert master */ -while (1 ) { - ret =wait_event_interruptible_timeout (mle->wq, - (atomic_read (&mle->woken) ==1 ), -msecs_to_jiffies (5000 )); - -if (ret >=0 ) { -if (atomic_read (&mle->woken) ==1 || - res->owner == target) -break ; - -mlog (0 ,"%s:%.*s: timed out during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); -/* avoid hang during shutdown when migrating lockres -* to a node which also goes down */ -if (dlm_is_node_dead (dlm, target)) { -mlog (0 ,"%s:%.*s: expected migration " -"target %u is no longer up, restarting\n" , - dlm->name, res->lockname.len, - res->lockname.name, target); - ret = -EINVAL ; -/* migration failed, detach and clean up mle */ -dlm_mle_detach_hb_events (dlm, mle); -dlm_put_mle (mle); -dlm_put_mle_inuse (mle); -spin_lock (&res->spinlock); - res->state &= ~DLM_LOCK_RES_MIGRATING ; - wake =1 ; -spin_unlock (&res->spinlock); - goto leave; - } - }else -mlog (0 ,"%s:%.*s: caught signal during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); +wait_event (mle->wq, ((atomic_read (&mle->woken) ==1 ) || + (atomic_read (&mle->woken) ==2 ))); + +/* migrate target down */ +if (atomic_read (&mle->woken) ==2 ) { +mlog (0 ,"%s:%.*s: expected migration " +"target %u is no longer up, restarting\n" , + dlm->name, res->lockname.len, + res->lockname.name, target); + ret = -EINVAL ; +dlm_put_mle_inuse (mle); +spin_lock (&res->spinlock); + res->state &= ~DLM_LOCK_RES_MIGRATING ; + wake =1 ; +spin_unlock (&res->spinlock); + goto leave; }/* all done, set the owner, clear the flag */ @@ -3227,7 +3210,7 @@ static void dlm_clean_migration_mle(struct dlm_ctxt *dlm,spin_lock (&mle->spinlock);__dlm_unlink_mle (dlm, mle); -atomic_set (&mle->woken,1 ); +atomic_set (&mle->woken,2 );spin_unlock (&mle->spinlock);
wake_up(&mle->wq);Thanks,
Junxiao.Quoted 150 lines
Thanks
Jiufei.Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
.
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Hi Jiufei,
Quoted 11 lines · xuejiufei
Hi Junxiao,
Quoted 9 lines · Junxiao Bi
Quoted 5 lines · xuejiufei
Hi Junxiao,
You are right. But it may happen thatmle->wokenis set to 1 indlm_clean_migration_mle()just afteratomic_read()indlm_migrate_lockres(). Actually we trigger this BUG whendlm_send_one_lockres()return error.Yes, that's possible because of that 5s timeout wakeup, I think this timeout is useless and can be removed?
Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
And I think
dlm_migrate_lockres()should not set owner to target and return 0 whenmle->wokenis set to 1 indlm_clean_migration_mle(). This is another problem?Yes, it should be fixed, or the lockres owner will be set to a down node wrongly. Can the following fix these two issue?
It can not fix the first issue when
dlm_send_one_lockres()ordlm_mark_lockres_migrating()return error, right?
Right. This is for second issue. Any way please consider these two issue, I think they can be fixed in one patch since both happened when target node down during migrate. You can make your own one or merge this patch to yours if you like.
Thanks,
Junxiao.
Quoted 232 lines
Quoted 231 lines
fs/ocfs2/dlm/dlmmaster.c +17
-34@@ -2618,39 +2618,22 @@ fail:/* wait for new node to assert master */ -while (1 ) { - ret =wait_event_interruptible_timeout (mle->wq, - (atomic_read (&mle->woken) ==1 ), -msecs_to_jiffies (5000 )); - -if (ret >=0 ) { -if (atomic_read (&mle->woken) ==1 || - res->owner == target) -break ; - -mlog (0 ,"%s:%.*s: timed out during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); -/* avoid hang during shutdown when migrating lockres -* to a node which also goes down */ -if (dlm_is_node_dead (dlm, target)) { -mlog (0 ,"%s:%.*s: expected migration " -"target %u is no longer up, restarting\n" , - dlm->name, res->lockname.len, - res->lockname.name, target); - ret = -EINVAL ; -/* migration failed, detach and clean up mle */ -dlm_mle_detach_hb_events (dlm, mle); -dlm_put_mle (mle); -dlm_put_mle_inuse (mle); -spin_lock (&res->spinlock); - res->state &= ~DLM_LOCK_RES_MIGRATING ; - wake =1 ; -spin_unlock (&res->spinlock); - goto leave; - } - }else -mlog (0 ,"%s:%.*s: caught signal during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); +wait_event (mle->wq, ((atomic_read (&mle->woken) ==1 ) || + (atomic_read (&mle->woken) ==2 ))); + +/* migrate target down */ +if (atomic_read (&mle->woken) ==2 ) { +mlog (0 ,"%s:%.*s: expected migration " +"target %u is no longer up, restarting\n" , + dlm->name, res->lockname.len, + res->lockname.name, target); + ret = -EINVAL ; +dlm_put_mle_inuse (mle); +spin_lock (&res->spinlock); + res->state &= ~DLM_LOCK_RES_MIGRATING ; + wake =1 ; +spin_unlock (&res->spinlock); + goto leave; }/* all done, set the owner, clear the flag */ @@ -3227,7 +3210,7 @@ static void dlm_clean_migration_mle(struct dlm_ctxt *dlm,spin_lock (&mle->spinlock);__dlm_unlink_mle (dlm, mle); -atomic_set (&mle->woken,1 ); +atomic_set (&mle->woken,2 );spin_unlock (&mle->spinlock);
wake_up(&mle->wq);Thanks,
Junxiao.Quoted 150 lines
Thanks
Jiufei.Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
.
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Hi Junxiao,
Function dlm_clean_migration_mle() will not be called after the first patch applied. So the issue that the owner of lockres will be set to a down node is not exist.
Thanks,
Jiufei
Quoted 264 lines · Junxiao Bi
Hi Jiufei,
Quoted 11 lines · xuejiufei
Hi Junxiao,
Quoted 9 lines · Junxiao Bi
Quoted 5 lines · xuejiufei
Hi Junxiao,
You are right. But it may happen thatmle->wokenis set to 1 indlm_clean_migration_mle()just afteratomic_read()indlm_migrate_lockres(). Actually we trigger this BUG whendlm_send_one_lockres()return error.Yes, that's possible because of that 5s timeout wakeup, I think this timeout is useless and can be removed?
Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
And I think
dlm_migrate_lockres()should not set owner to target and return 0 whenmle->wokenis set to 1 indlm_clean_migration_mle(). This is another problem?Yes, it should be fixed, or the lockres owner will be set to a down node wrongly. Can the following fix these two issue?
It can not fix the first issue when
dlm_send_one_lockres()ordlm_mark_lockres_migrating()return error, right?Right. This is for second issue. Any way please consider these two issue, I think they can be fixed in one patch since both happened when target node down during migrate. You can make your own one or merge this patch to yours if you like.
Thanks,
Junxiao.Quoted 229 lines
Quoted 229 lines
fs/ocfs2/dlm/dlmmaster.c +17
-34@@ -2618,39 +2618,22 @@ fail:/* wait for new node to assert master */ -while (1 ) { - ret =wait_event_interruptible_timeout (mle->wq, - (atomic_read (&mle->woken) ==1 ), -msecs_to_jiffies (5000 )); - -if (ret >=0 ) { -if (atomic_read (&mle->woken) ==1 || - res->owner == target) -break ; - -mlog (0 ,"%s:%.*s: timed out during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); -/* avoid hang during shutdown when migrating lockres -* to a node which also goes down */ -if (dlm_is_node_dead (dlm, target)) { -mlog (0 ,"%s:%.*s: expected migration " -"target %u is no longer up, restarting\n" , - dlm->name, res->lockname.len, - res->lockname.name, target); - ret = -EINVAL ; -/* migration failed, detach and clean up mle */ -dlm_mle_detach_hb_events (dlm, mle); -dlm_put_mle (mle); -dlm_put_mle_inuse (mle); -spin_lock (&res->spinlock); - res->state &= ~DLM_LOCK_RES_MIGRATING ; - wake =1 ; -spin_unlock (&res->spinlock); - goto leave; - } - }else -mlog (0 ,"%s:%.*s: caught signal during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); +wait_event (mle->wq, ((atomic_read (&mle->woken) ==1 ) || + (atomic_read (&mle->woken) ==2 ))); + +/* migrate target down */ +if (atomic_read (&mle->woken) ==2 ) { +mlog (0 ,"%s:%.*s: expected migration " +"target %u is no longer up, restarting\n" , + dlm->name, res->lockname.len, + res->lockname.name, target); + ret = -EINVAL ; +dlm_put_mle_inuse (mle); +spin_lock (&res->spinlock); + res->state &= ~DLM_LOCK_RES_MIGRATING ; + wake =1 ; +spin_unlock (&res->spinlock); + goto leave; }/* all done, set the owner, clear the flag */ @@ -3227,7 +3210,7 @@ static void dlm_clean_migration_mle(struct dlm_ctxt *dlm,spin_lock (&mle->spinlock);__dlm_unlink_mle (dlm, mle); -atomic_set (&mle->woken,1 ); +atomic_set (&mle->woken,2 );spin_unlock (&mle->spinlock);
wake_up(&mle->wq);Thanks,
Junxiao.Quoted 150 lines
Thanks
Jiufei.Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
Quoted 3 lines
Quoted 2 lines
.
.
PATCH 1/1 ocfs2/dlm: ignore cleaning the migration mle that is inuse
Quoted 4 lines · xuejiufei
Hi Junxiao,
Functiondlm_clean_migration_mle()will not be called after the first patch applied. So the issue that the owner of lockres will be set to a down node is not exist.
Right. Thank you for the explanation.
Thanks,
Junxiao.
Quoted 270 lines
Thanks,
JiufeiQuoted 264 lines · Junxiao Bi
Hi Jiufei,
Quoted 11 lines · xuejiufei
Hi Junxiao,
Quoted 9 lines · Junxiao Bi
Quoted 5 lines · xuejiufei
Hi Junxiao,
You are right. But it may happen thatmle->wokenis set to 1 indlm_clean_migration_mle()just afteratomic_read()indlm_migrate_lockres(). Actually we trigger this BUG whendlm_send_one_lockres()return error.Yes, that's possible because of that 5s timeout wakeup, I think this timeout is useless and can be removed?
Quoted 1 line
Quoted 7 lines
Quoted 5 lines
Quoted 2 lines
And I think
dlm_migrate_lockres()should not set owner to target and return 0 whenmle->wokenis set to 1 indlm_clean_migration_mle(). This is another problem?Yes, it should be fixed, or the lockres owner will be set to a down node wrongly. Can the following fix these two issue?
It can not fix the first issue when
dlm_send_one_lockres()ordlm_mark_lockres_migrating()return error, right?Right. This is for second issue. Any way please consider these two issue, I think they can be fixed in one patch since both happened when target node down during migrate. You can make your own one or merge this patch to yours if you like.
Thanks,
Junxiao.Quoted 229 lines
Quoted 229 lines
fs/ocfs2/dlm/dlmmaster.c +17
-34@@ -2618,39 +2618,22 @@ fail:/* wait for new node to assert master */ -while (1 ) { - ret =wait_event_interruptible_timeout (mle->wq, - (atomic_read (&mle->woken) ==1 ), -msecs_to_jiffies (5000 )); - -if (ret >=0 ) { -if (atomic_read (&mle->woken) ==1 || - res->owner == target) -break ; - -mlog (0 ,"%s:%.*s: timed out during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); -/* avoid hang during shutdown when migrating lockres -* to a node which also goes down */ -if (dlm_is_node_dead (dlm, target)) { -mlog (0 ,"%s:%.*s: expected migration " -"target %u is no longer up, restarting\n" , - dlm->name, res->lockname.len, - res->lockname.name, target); - ret = -EINVAL ; -/* migration failed, detach and clean up mle */ -dlm_mle_detach_hb_events (dlm, mle); -dlm_put_mle (mle); -dlm_put_mle_inuse (mle); -spin_lock (&res->spinlock); - res->state &= ~DLM_LOCK_RES_MIGRATING ; - wake =1 ; -spin_unlock (&res->spinlock); - goto leave; - } - }else -mlog (0 ,"%s:%.*s: caught signal during migration\n" , - dlm->name, res->lockname.len, res->lockname.name); +wait_event (mle->wq, ((atomic_read (&mle->woken) ==1 ) || + (atomic_read (&mle->woken) ==2 ))); + +/* migrate target down */ +if (atomic_read (&mle->woken) ==2 ) { +mlog (0 ,"%s:%.*s: expected migration " +"target %u is no longer up, restarting\n" , + dlm->name, res->lockname.len, + res->lockname.name, target); + ret = -EINVAL ; +dlm_put_mle_inuse (mle); +spin_lock (&res->spinlock); + res->state &= ~DLM_LOCK_RES_MIGRATING ; + wake =1 ; +spin_unlock (&res->spinlock); + goto leave; }/* all done, set the owner, clear the flag */ @@ -3227,7 +3210,7 @@ static void dlm_clean_migration_mle(struct dlm_ctxt *dlm,spin_lock (&mle->spinlock);__dlm_unlink_mle (dlm, mle); -atomic_set (&mle->woken,1 ); +atomic_set (&mle->woken,2 );spin_unlock (&mle->spinlock);
wake_up(&mle->wq);Thanks,
Junxiao.Quoted 150 lines
Thanks
Jiufei.Quoted 144 lines · Junxiao Bi
Hi Jiufei,
When target node down, mle is cleared from dlm_do_local_recovery_cleanup()->dlm_clean_master_list()->dlm_clean_migration_mle()?
mle->wokenis set to 1 indlm_clean_migration_mle(), so the code to detect target nodedown(if (dlm_is_node_dead(dlm, target)))will never be run indlm_migrate_lockres()?2621 while (1) {
2622 ret = wait_event_interruptible_timeout(mle->wq,
2623 (atomic_read(&mle->woken)
== 1),
2624msecs_to_jiffies(5000));
2625
2626 if (ret >= 0) {
2627 if (atomic_read(&mle->woken)== 1 ||
2628res->owner== target)
2629 break;
2630
2631 mlog(0, "%s:%.*s: timed out during
migration\n",
2632dlm->name,res->lockname.len,res->lockname.name);
2633 /* avoid hang during shutdown when
migrating lockres
2634 * to a node which also goes down */
2635 if (dlm_is_node_dead(dlm, target)) {
2636 mlog(0, "%s:%.*s: expected migration "
2637 "target %u is no longer up,
restarting\n",
2638dlm->name,res->lockname.len,
2639res->lockname.name, target);
2640 ret =-EINVAL;
2641 /* migration failed, detach and
clean up mle */2642 dlm_mle_detach_hb_events(dlm, mle); 2643 dlm_put_mle(mle); 2644 dlm_put_mle_inuse(mle); 2645 spin_lock(&res->spinlock); 2646 res->state &= ~DLM_LOCK_RES_MIGRATING; 2647 wake = 1; 2648 spin_unlock(&res->spinlock); 2649 goto leave;2650 }
2651 } else
2652 mlog(0, "%s:%.*s: caught signal during
migration\n",
2653dlm->name,res->lockname.len,res->lockname.name);
2654 }Thanks,
Junxiao.Quoted 74 lines · xuejiufei
We have found that migration source will trigger a BUG that the refcount of mle is already zero before put when the target is down during migration. The situation is as follows:
dlm_migrate_lockres
dlm_add_migration_mle
dlm_mark_lockres_migrating
dlm_get_mle_inuse
<<<<<< Now the refcount of the mle is 2.dlm_send_one_lockresand wait for the target to become the new master.
<<<<<< o2hb detect the target down and clean the migration mle. Now the refcount is 1.
dlm_migrate_lockreswoken, and put the mle twice when found the target goes down which trigger the BUG with the following message:
"ERROR: bad mle: ".Signed-off-by Jiufei Xue <[email protected]>Reviewed-by Joseph Qi <[email protected]>--- fs/ocfs2/dlm/dlmmaster.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)fs/ocfs2/dlm/dlmmaster.c +8
-11@@ -2519,6 +2519,11 @@ static int dlm_migrate_lockres(struct dlm_ctxt *dlm,spin_lock (&dlm->master_lock); ret =dlm_add_migration_mle (dlm, res, mle, &oldmle, name, namelen, target, dlm->node_num); +/* get an extra reference on the mle. +* otherwise the assert_master from the new +* master will destroy this. +*/ +dlm_get_mle_inuse (mle);spin_unlock (&dlm->master_lock);spin_unlock (&dlm->spinlock);@@ -2554,6 +2559,7 @@ fail:if (mle_added) {dlm_mle_detach_hb_events (dlm, mle);dlm_put_mle (mle); +dlm_put_mle_inuse (mle); }else if (mle) {kmem_cache_free (dlm_mle_cache, mle); mle =NULL ;@@ -2571,17 +2577,6 @@ fail:* ensure that all assert_master work is flushed. */ flush_workqueue (dlm->dlm_worker); -/* get an extra reference on the mle. -* otherwise the assert_master from the new -* master will destroy this. -* also, make sure that all callers of dlm_get_mle -* take both dlm->spinlock and dlm->master_lock */ -spin_lock (&dlm->spinlock); -spin_lock (&dlm->master_lock); -dlm_get_mle_inuse (mle); -spin_unlock (&dlm->master_lock); -spin_unlock (&dlm->spinlock); -/* notify new node and send all lock state */ /* call send_one_lockres with migration flag. * this serves as notice to the target node that a @@ -3312,6 +3307,15 @@ top: mle->new_master != dead_node)continue ; +if (mle->new_master == dead_node && mle->inuse) { + mlog(ML_NOTICE, "%s: target %u died during "fs/ocfs2/dlm/dlmmaster.c top: · line 3321
Quoted 10 lines
fs/ocfs2/dlm/dlmmaster.c +7
-0+"migration from %u, the MLE is " +"still keep used, ignore it!\n" , + dlm->name, dead_node, + mle->master); +continue ; + } +/* If we have reached this point, this mle needs to be * removed from the list and freed. */ dlm_clean_migration_mle (dlm, mle);.
Quoted 3 lines
Quoted 2 lines
.
.