ocfs2/dlm: don't handle migrate lockres if already in shutdown

PATCH New piaojun 3 participants 9 messages Mar 1, 2018 – Mar 2, 2018 OCFS2

Share & tools

piaojun

author

2018-03-01 09:26 UTC #0 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.

N1 N2 (owner)
touch file

access the file,
and get pr lock

umount

migrate all lockres

umount and migrate lockres to N1

leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Yiwen Jiang <[email protected]>

---
 fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
 fs/ocfs2/dlm/dlmdomain.h   |  1 +
 fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
 3 files changed, 24 insertions(+)

fs/ocfs2/dlm/dlmdomain.c +14 -0

index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
 	spin_unlock(&dlm->spinlock);
 }
 
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
 int dlm_shutting_down(struct dlm_ctxt *dlm)
 {
 	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
 extern spinlock_t dlm_domain_lock;
 extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
 int dlm_shutting_down(struct dlm_ctxt *dlm);
 void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
 					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +9 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
 	if (!dlm_grab(dlm))
 		return -EINVAL;
 
+	if (!dlm_joined(dlm)) {
+		mlog(ML_ERROR, "Domain %s not joined! "
+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
 	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
 	real_master = mres->master;

Changwei Ge

2018-03-01 09:39 UTC #1 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Hi Jun,

Quoted 5 lines · piaojun

We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.

Quoted 8 lines

N1 N2 (owner)
touch file
access the file,
and get pr lock
umount

Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.

How can the scenario your changelog describing happen? Or if miss something?

Thanks,
Changwei

Quoted 75 lines

migrate all lockres

umount and migrate lockres to N1

leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Yiwen Jiang <[email protected]>

---
  fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
  fs/ocfs2/dlm/dlmdomain.h   |  1 +
  fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
  3 files changed, 24 insertions(+)

fs/ocfs2/dlm/dlmdomain.c +14 -0

index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
  	spin_unlock(&dlm->spinlock);
  }
 
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
  int dlm_shutting_down(struct dlm_ctxt *dlm)
  {
  	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
  extern spinlock_t dlm_domain_lock;
  extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
  int dlm_shutting_down(struct dlm_ctxt *dlm);
  void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
  					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +9 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
  	if (!dlm_grab(dlm))
  		return -EINVAL;
 
+	if (!dlm_joined(dlm)) {
+		mlog(ML_ERROR, "Domain %s not joined! "
+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
  	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
  	real_master = mres->master;

piaojun

author

2018-03-01 12:37 UTC #2 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Hi Changwei,

Thanks for your quick reply, please see my comments below.

Quoted 14 lines · Changwei Ge

Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock

Quoted 7 lines

Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.

Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.

N1 N2 (owner)
touch file

access the file,
and get pr lock

begin leave domain and pick up N1 as new owner

begin leave domain and
migrate all lockres done

begin migrate lockres to N1

end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed

thanks,
Jun

Quoted 84 lines

How can the scenario your changelog describing happen? Or if miss something?

Thanks,
Changwei

Quoted 65 lines

migrate all lockres

umount and migrate lockres to N1

leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Yiwen Jiang <[email protected]>

---
  fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
  fs/ocfs2/dlm/dlmdomain.h   |  1 +
  fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
  3 files changed, 24 insertions(+)

fs/ocfs2/dlm/dlmdomain.c +14 -0

index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
  	spin_unlock(&dlm->spinlock);
  }
 
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
  int dlm_shutting_down(struct dlm_ctxt *dlm)
  {
  	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
  extern spinlock_t dlm_domain_lock;
  extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
  int dlm_shutting_down(struct dlm_ctxt *dlm);
  void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
  					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +2 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
  	if (!dlm_grab(dlm))
  		return -EINVAL;
 
+	if (!dlm_joined(dlm)) {
+		mlog(ML_ERROR, "Domain %s not joined! "

fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1392

Quoted 10 lines

fs/ocfs2/dlm/dlmrecovery.c +7 -0

+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
  	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
  	real_master = mres->master;

Andrew Morton

2018-03-01 23:29 UTC #3 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Quoted 43 lines · On Thu, 1 Mar 2018 20:37:50 +0800 piaojun <[email protected]> wrote:

Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file
access the file,
and get pr lock
begin leave domain and pick up N1 as new owner
begin leave domain and
migrate all lockres done

Quoted 6 lines

begin migrate lockres to N1
end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed

If someone asked a question then this is a sign that the changelog was missing details. So please do send along a v2 with a more comprehensive changelog.

piaojun

author

2018-03-02 09:40 UTC #4 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Hi Andrew,

Thanks for your suggestion, I will give a more comprehensive changelog in patch v2 later.

thanks,
Jun

Quoted 58 lines · Andrew Morton

Quoted 43 lines · On Thu, 1 Mar 2018 20:37:50 +0800 piaojun <[email protected]> wrote:
Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file
access the file,
and get pr lock
begin leave domain and pick up N1 as new owner
begin leave domain and
migrate all lockres done
Quoted 6 lines
begin migrate lockres to N1
end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed
If someone asked a question then this is a sign that the changelog was missing details. So please do send along a v2 with a more comprehensive changelog.
.

Changwei Ge

2018-03-02 01:49 UTC #5 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

In reply to piaojun

Hi Jun,
I still have some doubts about your problematic situation description . Please check my reply inline your sequence diagram.

Quoted 34 lines · piaojun

Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file

Quoted 7 lines

access the file,
and get pr lock
begin leave domain and pick up N1 as new owner
begin leave domain and

Here will clear N1 from N2's dlm->domain_map

Quoted 2 lines

migrate all lockres do >
begin migrate lockres to N1

As N1 has been cleared, migrating for this lock resource can't continue.

Besides,
I wonder what kind of problem you encountered?
A bug crash? A hang situation?
It's better to share some issue appearance.So perhaps I can help to analyze.:)

Thanks,
Changwei

Quoted 93 lines

end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed

thanks,
Jun

Quoted 84 lines

How can the scenario your changelog describing happen? Or if miss something?

Thanks,
Changwei

Quoted 65 lines

migrate all lockres

umount and migrate lockres to N1

leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Yiwen Jiang <[email protected]>

---
   fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
   fs/ocfs2/dlm/dlmdomain.h   |  1 +
   fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
   3 files changed, 24 insertions(+)

fs/ocfs2/dlm/dlmdomain.c +14 -0

index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
   	spin_unlock(&dlm->spinlock);
   }
 
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
   int dlm_shutting_down(struct dlm_ctxt *dlm)
   {
   	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
   extern spinlock_t dlm_domain_lock;
   extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
   int dlm_shutting_down(struct dlm_ctxt *dlm);
   void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
   					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +2 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
   	if (!dlm_grab(dlm))
   		return -EINVAL;
 
+	if (!dlm_joined(dlm)) {
+		mlog(ML_ERROR, "Domain %s not joined! "

fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1392

Quoted 10 lines

fs/ocfs2/dlm/dlmrecovery.c +7 -0

+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
   	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
   	real_master = mres->master;

piaojun

author

2018-03-02 02:08 UTC #6 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Hi Changwei,

Quoted 45 lines · Changwei Ge

Hi Jun,
I still have some doubts about your problematic situation description . Please check my reply inline your sequence diagram.
Quoted 40 lines · piaojun
Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file
access the file,
and get pr lock
begin leave domain and pick up N1 as new owner

Quoted 6 lines

Quoted 1 line
begin leave domain and
Here will clear N1 from N2's dlm->domain_map
Quoted 2 lines
migrate all lockres do >
begin migrate lockres to N1
As N1 has been cleared, migrating for this lock resource can't continue.

N2 has picked up N1 as new owner in dlm_pick_migration_target() before setting N1 in N2's dlm->exit_domain_map, so N2 will continue migrating lock resource.

Quoted 5 lines

Besides,
I wonder what kind of problem you encountered?
A bug crash? A hang situation?
It's better to share some issue appearance.So perhaps I can help to analyze.:)

I encountered a stuck case which described in my changelog.

"
At last other nodes will get stuck into infinite loop when requsting lock from us.
"

thanks,
Jun

Quoted 99 lines

Thanks,
Changwei

Quoted 93 lines

end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed

thanks,
Jun

Quoted 84 lines

How can the scenario your changelog describing happen? Or if miss something?

Thanks,
Changwei

Quoted 65 lines

migrate all lockres

umount and migrate lockres to N1

leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed

Signed-off-by Jun Piao <[email protected]>

Reviewed-by Yiwen Jiang <[email protected]>

---
   fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
   fs/ocfs2/dlm/dlmdomain.h   |  1 +
   fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
   3 files changed, 24 insertions(+)

fs/ocfs2/dlm/dlmdomain.c +14 -0

index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
   	spin_unlock(&dlm->spinlock);
   }
 
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
   int dlm_shutting_down(struct dlm_ctxt *dlm)
   {
   	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
   extern spinlock_t dlm_domain_lock;
   extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
   int dlm_shutting_down(struct dlm_ctxt *dlm);
   void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
   					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +2 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
   	if (!dlm_grab(dlm))
   		return -EINVAL;
 
+	if (!dlm_joined(dlm)) {
+		mlog(ML_ERROR, "Domain %s not joined! "

fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1392

Quoted 10 lines

fs/ocfs2/dlm/dlmrecovery.c +7 -0

+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
   	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
   	real_master = mres->master;

Changwei Ge

2018-03-02 05:53 UTC #7 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Quoted 49 lines · piaojun

Hi Changwei,
Quoted 46 lines · Changwei Ge
Hi Jun,
I still have some doubts about your problematic situation description . Please check my reply inline your sequence diagram.
Quoted 41 lines · piaojun
Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file
access the file,
and get pr lock
begin leave domain and pick up N1 as new owner
begin leave domain and

Quoted 8 lines

Quoted 5 lines
Here will clear N1 from N2's dlm->domain_map
Quoted 2 lines
migrate all lockres do >
begin migrate lockres to N1
As N1 has been cleared, migrating for this lock resource can't continue.
N2 has picked up N1 as new owner in dlm_pick_migration_target() before setting N1 in N2's dlm->exit_domain_map, so N2 will continue migrating lock resource.

Do you mean that DLM_BEGIN_EXIT_DOMAIN_MSG arrives just before dlm_send_one_lockres() in dlm_migrate_lockres() and after dlm_mark_lockres_migrating()?
If so there is race that your problem can happen.

Well let's move to your code.

Quoted 58 lines

Quoted 5 lines
Besides,
I wonder what kind of problem you encountered?
A bug crash? A hang situation?
It's better to share some issue appearance.So perhaps I can help to analyze.:)
I encountered a stuck case which described in my changelog.
"
At last other nodes will get stuck into infinite loop when requsting lock from us.
"
thanks,
Jun
Quoted 44 lines
Thanks,
Changwei
Quoted 40 lines
end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed
thanks,
Jun
Quoted 32 lines
How can the scenario your changelog describing happen? Or if miss something?
Thanks,
Changwei
Quoted 25 lines
migrate all lockres
umount and migrate lockres to N1
leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Yiwen Jiang <[email protected]>
---
    fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
    fs/ocfs2/dlm/dlmdomain.h   |  1 +
    fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
    3 files changed, 24 insertions(+)
fs/ocfs2/dlm/dlmdomain.c +0 -0
index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
    	spin_unlock(&dlm->spinlock);
    }
 

Quoted 14 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
fs/ocfs2/dlm/dlmdomain.c +6 -0
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
fs/ocfs2/dlm/dlmdomain.c static void dlm_leave_domain(struct dlm_ctxt *dlm) · line 691
Quoted 8 lines
fs/ocfs2/dlm/dlmdomain.c +8 -0
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+

How about use directly use existed function dlm_shutting_down()

Quoted 23 lines

fs/ocfs2/dlm/dlmdomain.c +0 -0

    int dlm_shutting_down(struct dlm_ctxt *dlm)
    {
    	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
    extern spinlock_t dlm_domain_lock;
    extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
    int dlm_shutting_down(struct dlm_ctxt *dlm);
    void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
    					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +0 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
    	if (!dlm_grab(dlm))
    		return -EINVAL;

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1381
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +1 -0
+	if (!dlm_joined(dlm)) {

Here can be if (dlm_shutting_down(dlm)) ?

Quoted 1 line

Quoted 1 line
Quoted 1 line
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1382
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +1 -0
+		mlog(ML_ERROR, "Domain %s not joined! "

The log is not accurate. Domain is shutting down will be better, I suppose.

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1384
Quoted 2 lines
fs/ocfs2/dlm/dlmrecovery.c +2 -0
+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,

I don't like the alignment style.
How about?
mlog(ML_ERROR, "Domain %s not joined! "

      "lockres %.*s, master %u\n",

Thanks,
Changwei

Quoted 15 lines

Quoted 14 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
fs/ocfs2/dlm/dlmrecovery.c +5 -0
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
    	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
Quoted 5 lines
Quoted 4 lines
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1392
Quoted 2 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
    	real_master = mres->master;
.
.

piaojun

author

2018-03-02 09:37 UTC #8 lore raw

PATCH 1/1 ocfs2/dlm: don't handle migrate lockres if already in shutdown

Hi Changwei,

Quoted 54 lines · Changwei Ge

Quoted 53 lines · piaojun
Hi Changwei,
Quoted 50 lines · Changwei Ge
Hi Jun,
I still have some doubts about your problematic situation description . Please check my reply inline your sequence diagram.
Quoted 34 lines · piaojun
Hi Changwei,
Thanks for your quick reply, please see my comments below.
Quoted 14 lines · Changwei Ge
Hi Jun,
Quoted 11 lines · piaojun
We should not handle migrate lockres if we are already in 'DLM_CTXT_IN_SHUTDOWN', as that will cause lockres remains after leaving dlm domain. At last other nodes will get stuck into infinite loop when requsting lock from us.
N1 N2 (owner)
touch file
access the file,
and get pr lock
Quoted 7 lines
Quoted 2 lines
umount
Before migrating all lock resources, N1 should have already sent DLM_BEGIN_EXIT_DOMAIN_MSG in dlm_begin_exit_domain(). N2 will set ->exit_domain_map later.
So N2 can't take N1 as migration target.
Before receiveing N1's DLM_BEGIN_EXIT_DOMAIN_MSG, N2 has picked up N1 as the migrate target. So N2 will continue sending lockres to N1 even though N1 has left domain. Sorry for making you misunderstanding, I will give a more detailed description.
N1 N2 (owner)
touch file
Quoted 7 lines
access the file,
and get pr lock
begin leave domain and pick up N1 as new owner
begin leave domain and
Here will clear N1 from N2's dlm->domain_map
Quoted 2 lines
migrate all lockres do >
begin migrate lockres to N1

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
As N1 has been cleared, migrating for this lock resource can't continue.
N2 has picked up N1 as new owner in dlm_pick_migration_target() before setting N1 in N2's dlm->exit_domain_map, so N2 will continue migrating lock resource.
Do you mean that DLM_BEGIN_EXIT_DOMAIN_MSG arrives just before dlm_send_one_lockres() in dlm_migrate_lockres() and after dlm_mark_lockres_migrating()?

Yes, I meant that.

Quoted 62 lines

If so there is race that your problem can happen.
Well let's move to your code.
Quoted 58 lines
Quoted 5 lines
Besides,
I wonder what kind of problem you encountered?
A bug crash? A hang situation?
It's better to share some issue appearance.So perhaps I can help to analyze.:)
I encountered a stuck case which described in my changelog.
"
At last other nodes will get stuck into infinite loop when requsting lock from us.
"
thanks,
Jun
Quoted 44 lines
Thanks,
Changwei
Quoted 40 lines
end leave domain, but
the lockres left
unexpectedly, because
migrate task has passed
thanks,
Jun
Quoted 32 lines
How can the scenario your changelog describing happen? Or if miss something?
Thanks,
Changwei
Quoted 25 lines
migrate all lockres
umount and migrate lockres to N1
leave dlm domain, but
the lockres left
unexpectedly, because
migrate task has passed
Signed-off-by Jun Piao <[email protected]>
Reviewed-by Yiwen Jiang <[email protected]>
---
    fs/ocfs2/dlm/dlmdomain.c   | 14 ++++++++++++++
    fs/ocfs2/dlm/dlmdomain.h   |  1 +
    fs/ocfs2/dlm/dlmrecovery.c |  9 +++++++++
    3 files changed, 24 insertions(+)
fs/ocfs2/dlm/dlmdomain.c +0 -0
index e1fea14..3b7ec51 100644
@@ -675,6 +675,20 @@ static void dlm_leave_domain(struct dlm_ctxt *dlm)
    	spin_unlock(&dlm->spinlock);
    }
 

Quoted 15 lines

Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
fs/ocfs2/dlm/dlmdomain.c +6 -0
+int dlm_joined(struct dlm_ctxt *dlm)
+{
+	int ret = 0;
+
+	spin_lock(&dlm_domain_lock);
+
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
fs/ocfs2/dlm/dlmdomain.c static void dlm_leave_domain(struct dlm_ctxt *dlm) · line 691
Quoted 8 lines
fs/ocfs2/dlm/dlmdomain.c +8 -0
+	if (dlm->dlm_state == DLM_CTXT_JOINED)
+		ret = 1;
+
+	spin_unlock(&dlm_domain_lock);
+
+	return ret;
+}
+
How about use directly use existed function dlm_shutting_down()

I'm afraid that we can't use dlm_shutting_down() directly, as dlm->dlm_state will be set DLM_CTXT_LEAVING in dlm_mark_domain_leaving() after N1 migrate all lockres.

Quoted 24 lines

fs/ocfs2/dlm/dlmdomain.c +0 -0

    int dlm_shutting_down(struct dlm_ctxt *dlm)
    {
    	int ret = 0;

fs/ocfs2/dlm/dlmdomain.h +1 -0

index fd6122a..2f7f60b 100644
@@ -28,6 +28,7 @@
    extern spinlock_t dlm_domain_lock;
    extern struct list_head dlm_domains;
 
+int dlm_joined(struct dlm_ctxt *dlm);
    int dlm_shutting_down(struct dlm_ctxt *dlm);
    void dlm_fire_domain_eviction_callbacks(struct dlm_ctxt *dlm,
    					int node_num);

fs/ocfs2/dlm/dlmrecovery.c +0 -0

index ec8f758..9b3bc66 100644
@@ -1378,6 +1378,15 @@ int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data,
    	if (!dlm_grab(dlm))
    		return -EINVAL;

Quoted 4 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1381
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +1 -0
+	if (!dlm_joined(dlm)) {
Here can be if (dlm_shutting_down(dlm)) ?
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1382
Quoted 1 line
fs/ocfs2/dlm/dlmrecovery.c +1 -0
+		mlog(ML_ERROR, "Domain %s not joined! "
The log is not accurate. Domain is shutting down will be better, I suppose.

Actually domain is in DLM_CTXT_IN_SHUTDOWN or DLM_CTXT_LEAVING, so we can hardly distinguish here. So we can only judge domain is not in DLM_CTXT_JOINED.

Quoted 7 lines

Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1384
Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c +2 -0
+				"lockres %.*s, master %u\n",
+				dlm->name, mres->lockname_len,
I don't like the alignment style.
How about?
mlog(ML_ERROR, "Domain %s not joined! "
      "lockres %.*s, master %u\n",

OK, I will fix this.

thanks,
Jun

Quoted 22 lines

Thanks,
Changwei
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
fs/ocfs2/dlm/dlmrecovery.c +5 -0
+				mres->lockname, mres->master);
+		dlm_put(dlm);
+		return -EINVAL;
+	}
+
    	BUG_ON(!(mres->flags & (DLM_MRES_RECOVERY|DLM_MRES_MIGRATION)));
 
Quoted 8 lines
Quoted 7 lines
Quoted 5 lines
Quoted 4 lines
fs/ocfs2/dlm/dlmrecovery.c int dlm_mig_lockres_handler(struct o2net_msg *msg, u32 len, void *data, · line 1392
Quoted 2 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
    	real_master = mres->master;
.
.
.