Skip to contentSkip to conversation

ocfs2: add uuid to ocfs2 thread name for problem analysis

PATCH New Joseph Qi 2 participants 6 messages Oct 27, 2015 – Oct 28, 2015 OCFS2

lore
Share & tools
lore

Joseph Qi

author
#0 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

A node can mount multiple ocfs2 volumes. And if thread names are same for each volume/domain, it will bring inconvenience when analyzing problems because we have to identify which volume/domain the messages belong to.
Since thread name will be printed to messages, so add volume uuid or dlm name to thread name (like o2hb thread) can benefit problem analysis.

Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmdomain.c   | 5 ++++-
 fs/ocfs2/dlm/dlmrecovery.c | 2 +-
 fs/ocfs2/dlm/dlmthread.c   | 3 ++-
 fs/ocfs2/dlmglue.c         | 3 ++-
 fs/ocfs2/journal.c         | 4 ++--
 5 files changed, 11 insertions(+), 6 deletions(-)
fs/ocfs2/dlm/dlmdomain.c +4 -1
index 7df88a6..9416124 100644
@@ -1860,6 +1860,7 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 	int status;
 	unsigned int backoff;
 	unsigned int total_backoff = 0;
+	char wq_name[O2NM_MAX_NAME_LEN];
 
 	BUG_ON(!dlm);
 

@@ -1889,7 +1890,9 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 		goto bail;
 	}
 
-	dlm->dlm_worker = create_singlethread_workqueue("dlm_wq");
+	memset(wq_name, 0, O2NM_MAX_NAME_LEN);
+	snprintf(wq_name, O2NM_MAX_NAME_LEN, "dlm_wq-%s", dlm->name);
+	dlm->dlm_worker = create_singlethread_workqueue(wq_name);
 	if (!dlm->dlm_worker) {
 		status = -ENOMEM;
 		mlog_errno(status);
fs/ocfs2/dlm/dlmrecovery.c +1 -1
index a43f9ef..570509e 100644
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
 
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);
 	if (IS_ERR(dlm->dlm_reco_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_reco_thread_task));
 		dlm->dlm_reco_thread_task = NULL;
fs/ocfs2/dlm/dlmthread.c +2 -1
index 69aac6f..e2bd691 100644
@@ -483,7 +483,8 @@ int dlm_launch_thread(struct dlm_ctxt *dlm)
 {
 	mlog(0, "Starting dlm_thread...\n");
 
-	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread");
+	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread-%s",
+			dlm->name);
 	if (IS_ERR(dlm->dlm_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_thread_task));
 		dlm->dlm_thread_task = NULL;
fs/ocfs2/dlmglue.c +2 -1
index 23157e4..fd4f536 100644
@@ -2998,7 +2998,8 @@ int ocfs2_dlm_init(struct ocfs2_super *osb)
 	}
 
 	/* launch downconvert thread */
-	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc");
+	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc-%s",
+			osb->uuid_str);
 	if (IS_ERR(osb->dc_task)) {
 		status = PTR_ERR(osb->dc_task);
 		osb->dc_task = NULL;
fs/ocfs2/journal.c +2 -2
index 627d88c..86b9a93 100644
@@ -1074,7 +1074,7 @@ int ocfs2_journal_load(struct ocfs2_journal *journal, int local, int replayed)
 	/* Launch the commit thread */
 	if (!local) {
 		osb->commit_task = kthread_run(ocfs2_commit_thread, osb,
-					       "ocfs2cmt");
+				"ocfs2cmt-%s", osb->uuid_str);
 		if (IS_ERR(osb->commit_task)) {
 			status = PTR_ERR(osb->commit_task);
 			osb->commit_task = NULL;

@@ -1491,7 +1491,7 @@ void ocfs2_recovery_thread(struct ocfs2_super *osb, int node_num)
 		goto out;
 
 	osb->recovery_thread_task =  kthread_run(__ocfs2_recovery_thread, osb,
-						 "ocfs2rec");
+			"ocfs2rec-%s", osb->uuid_str);
 	if (IS_ERR(osb->recovery_thread_task)) {
 		mlog_errno((int)PTR_ERR(osb->recovery_thread_task));
 		osb->recovery_thread_task = NULL;
Signature · 1 line
1.8.4.3
#1 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

Hi Joseph,

See comments below.

Quoted 29 lines · Joseph Qi

A node can mount multiple ocfs2 volumes. And if thread names are same for each volume/domain, it will bring inconvenience when analyzing problems because we have to identify which volume/domain the messages belong to.
Since thread name will be printed to messages, so add volume uuid or dlm name to thread name (like o2hb thread) can benefit problem analysis.

Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmdomain.c   | 5 ++++-
 fs/ocfs2/dlm/dlmrecovery.c | 2 +-
 fs/ocfs2/dlm/dlmthread.c   | 3 ++-
 fs/ocfs2/dlmglue.c         | 3 ++-
 fs/ocfs2/journal.c         | 4 ++--
 5 files changed, 11 insertions(+), 6 deletions(-)
fs/ocfs2/dlm/dlmdomain.c +1 -0
index 7df88a6..9416124 100644
@@ -1860,6 +1860,7 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 	int status;
 	unsigned int backoff;
 	unsigned int total_backoff = 0;
+	char wq_name[O2NM_MAX_NAME_LEN];
 
 	BUG_ON(!dlm);

fs/ocfs2/dlm/dlmdomain.c static int dlm_join_domain(struct dlm_ctxt *dlm) · line 1893

Quoted 5 lines
fs/ocfs2/dlm/dlmdomain.c +1 -1
@@ -1889,7 +1890,9 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 		goto bail;
 	}
 
-	dlm->dlm_worker = create_singlethread_workqueue("dlm_wq");
+	memset(wq_name, 0, O2NM_MAX_NAME_LEN);

I didn't see memset need here.

Quoted 10 lines
fs/ocfs2/dlm/dlmdomain.c +2 -0
+	snprintf(wq_name, O2NM_MAX_NAME_LEN, "dlm_wq-%s", dlm->name);
+	dlm->dlm_worker = create_singlethread_workqueue(wq_name);
 	if (!dlm->dlm_worker) {
 		status = -ENOMEM;
 		mlog_errno(status);
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index a43f9ef..570509e 100644

fs/ocfs2/dlm/dlmrecovery.c int dlm_launch_recovery_thread(struct dlm_ctxt *dlm) · line 208

Quoted 5 lines
fs/ocfs2/dlm/dlmrecovery.c +1 -1
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
 
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);

Indeed max length of task name is 16 bytes, and "dlm_reco_thread" plus '\0' have taken all the space. So indeed above code is useless. Can we rename this name and maybe other one(like "dlm_thread") to leave more space for domain marker?

Thanks,
Junxiao.

Quoted 54 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
 	if (IS_ERR(dlm->dlm_reco_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_reco_thread_task));
 		dlm->dlm_reco_thread_task = NULL;
fs/ocfs2/dlm/dlmthread.c +2 -1
index 69aac6f..e2bd691 100644
@@ -483,7 +483,8 @@ int dlm_launch_thread(struct dlm_ctxt *dlm)
 {
 	mlog(0, "Starting dlm_thread...\n");
 
-	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread");
+	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread-%s",
+			dlm->name);
 	if (IS_ERR(dlm->dlm_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_thread_task));
 		dlm->dlm_thread_task = NULL;
fs/ocfs2/dlmglue.c +2 -1
index 23157e4..fd4f536 100644
@@ -2998,7 +2998,8 @@ int ocfs2_dlm_init(struct ocfs2_super *osb)
 	}
 
 	/* launch downconvert thread */
-	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc");
+	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc-%s",
+			osb->uuid_str);
 	if (IS_ERR(osb->dc_task)) {
 		status = PTR_ERR(osb->dc_task);
 		osb->dc_task = NULL;
fs/ocfs2/journal.c +2 -2
index 627d88c..86b9a93 100644
@@ -1074,7 +1074,7 @@ int ocfs2_journal_load(struct ocfs2_journal *journal, int local, int replayed)
 	/* Launch the commit thread */
 	if (!local) {
 		osb->commit_task = kthread_run(ocfs2_commit_thread, osb,
-					       "ocfs2cmt");
+				"ocfs2cmt-%s", osb->uuid_str);
 		if (IS_ERR(osb->commit_task)) {
 			status = PTR_ERR(osb->commit_task);
 			osb->commit_task = NULL;

@@ -1491,7 +1491,7 @@ void ocfs2_recovery_thread(struct ocfs2_super *osb, int node_num)
 		goto out;
 
 	osb->recovery_thread_task =  kthread_run(__ocfs2_recovery_thread, osb,
-						 "ocfs2rec");
+			"ocfs2rec-%s", osb->uuid_str);
 	if (IS_ERR(osb->recovery_thread_task)) {
 		mlog_errno((int)PTR_ERR(osb->recovery_thread_task));
 		osb->recovery_thread_task = NULL;
author
#2 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

Hi Junxiao,

Quoted 33 lines · Junxiao Bi

Hi Joseph,

See comments below.

Quoted 28 lines · Joseph Qi

A node can mount multiple ocfs2 volumes. And if thread names are same for each volume/domain, it will bring inconvenience when analyzing problems because we have to identify which volume/domain the messages belong to.
Since thread name will be printed to messages, so add volume uuid or dlm name to thread name (like o2hb thread) can benefit problem analysis.

Signed-off-by Joseph Qi <[email protected]>
---
 fs/ocfs2/dlm/dlmdomain.c   | 5 ++++-
 fs/ocfs2/dlm/dlmrecovery.c | 2 +-
 fs/ocfs2/dlm/dlmthread.c   | 3 ++-
 fs/ocfs2/dlmglue.c         | 3 ++-
 fs/ocfs2/journal.c         | 4 ++--
 5 files changed, 11 insertions(+), 6 deletions(-)
fs/ocfs2/dlm/dlmdomain.c +1 -0
index 7df88a6..9416124 100644
@@ -1860,6 +1860,7 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 	int status;
 	unsigned int backoff;
 	unsigned int total_backoff = 0;
+	char wq_name[O2NM_MAX_NAME_LEN];
 
 	BUG_ON(!dlm);
Quoted 8 lines

fs/ocfs2/dlm/dlmdomain.c static int dlm_join_domain(struct dlm_ctxt *dlm) · line 1893

Quoted 6 lines
fs/ocfs2/dlm/dlmdomain.c +1 -1
@@ -1889,7 +1890,9 @@ static int dlm_join_domain(struct dlm_ctxt *dlm)
 		goto bail;
 	}
 
-	dlm->dlm_worker = create_singlethread_workqueue("dlm_wq");
+	memset(wq_name, 0, O2NM_MAX_NAME_LEN);

I didn't see memset need here.

I will remove it since snprintf will include the trailing null byte.

Quoted 12 lines
Quoted 12 lines
fs/ocfs2/dlm/dlmdomain.c +2 -0
+	snprintf(wq_name, O2NM_MAX_NAME_LEN, "dlm_wq-%s", dlm->name);
+	dlm->dlm_worker = create_singlethread_workqueue(wq_name);
 	if (!dlm->dlm_worker) {
 		status = -ENOMEM;
 		mlog_errno(status);
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index a43f9ef..570509e 100644
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
Quoted 8 lines

fs/ocfs2/dlm/dlmrecovery.c int dlm_launch_recovery_thread(struct dlm_ctxt *dlm) · line 208

Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c +1 -1
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);

Indeed max length of task name is 16 bytes, and "dlm_reco_thread" plus '\0' have taken all the space. So indeed above code is useless. Can we rename this name and maybe other one(like "dlm_thread") to leave more space for domain marker?

Yes, you are right. For dlm_reco_thread it won't print any uuid bytes. I put it here just for code consistency.
It is really hard for me to rename it to a better one:)
Any suggestions?

Thanks,
Joseph

Quoted 61 lines

Thanks,
Junxiao.

Quoted 46 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
 	if (IS_ERR(dlm->dlm_reco_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_reco_thread_task));
 		dlm->dlm_reco_thread_task = NULL;
fs/ocfs2/dlm/dlmthread.c +2 -1
index 69aac6f..e2bd691 100644
@@ -483,7 +483,8 @@ int dlm_launch_thread(struct dlm_ctxt *dlm)
 {
 	mlog(0, "Starting dlm_thread...\n");
 
-	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread");
+	dlm->dlm_thread_task = kthread_run(dlm_thread, dlm, "dlm_thread-%s",
+			dlm->name);
 	if (IS_ERR(dlm->dlm_thread_task)) {
 		mlog_errno(PTR_ERR(dlm->dlm_thread_task));
 		dlm->dlm_thread_task = NULL;
fs/ocfs2/dlmglue.c +2 -1
index 23157e4..fd4f536 100644
@@ -2998,7 +2998,8 @@ int ocfs2_dlm_init(struct ocfs2_super *osb)
 	}
 
 	/* launch downconvert thread */
-	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc");
+	osb->dc_task = kthread_run(ocfs2_downconvert_thread, osb, "ocfs2dc-%s",
+			osb->uuid_str);
 	if (IS_ERR(osb->dc_task)) {
 		status = PTR_ERR(osb->dc_task);
 		osb->dc_task = NULL;
fs/ocfs2/journal.c +1 -1
index 627d88c..86b9a93 100644
@@ -1074,7 +1074,7 @@ int ocfs2_journal_load(struct ocfs2_journal *journal, int local, int replayed)
 	/* Launch the commit thread */
 	if (!local) {
 		osb->commit_task = kthread_run(ocfs2_commit_thread, osb,
-					       "ocfs2cmt");
+				"ocfs2cmt-%s", osb->uuid_str);
 		if (IS_ERR(osb->commit_task)) {
 			status = PTR_ERR(osb->commit_task);
 			osb->commit_task = NULL;

fs/ocfs2/journal.c void ocfs2_recovery_thread(struct ocfs2_super *osb, int node_num) · line 1497

Quoted 8 lines
fs/ocfs2/journal.c +1 -1
@@ -1491,7 +1491,7 @@ void ocfs2_recovery_thread(struct ocfs2_super *osb, int node_num)
 		goto out;
 
 	osb->recovery_thread_task =  kthread_run(__ocfs2_recovery_thread, osb,
-						 "ocfs2rec");
+			"ocfs2rec-%s", osb->uuid_str);
 	if (IS_ERR(osb->recovery_thread_task)) {
 		mlog_errno((int)PTR_ERR(osb->recovery_thread_task));
 		osb->recovery_thread_task = NULL;

.

#3 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

Quoted 2 lines · Joseph Qi

Hi Junxiao,

...

Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index a43f9ef..570509e 100644
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
Quoted 8 lines

fs/ocfs2/dlm/dlmrecovery.c int dlm_launch_recovery_thread(struct dlm_ctxt *dlm) · line 208

Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c +1 -1
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);

Indeed max length of task name is 16 bytes, and "dlm_reco_thread" plus '\0' have taken all the space. So indeed above code is useless. Can we rename this name and maybe other one(like "dlm_thread") to leave more space for domain marker?

Quoted 4 lines

Yes, you are right. For dlm_reco_thread it won't print any uuid bytes. I put it here just for code consistency.
It is really hard for me to rename it to a better one:)
Any suggestions?

How about this?

dlmwq-xxxx
dlmrec-xxxx
dlm-xxxx
o2dc-xxxx
o2cmt-xxx
o2rec-xxx

Thanks,
Junxiao.

Quoted 4 lines

Thanks,
Joseph

author
#4 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

Quoted 23 lines · Junxiao Bi
Quoted 2 lines · Joseph Qi

Hi Junxiao,

...

Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index a43f9ef..570509e 100644
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
Quoted 8 lines

fs/ocfs2/dlm/dlmrecovery.c int dlm_launch_recovery_thread(struct dlm_ctxt *dlm) · line 208

Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c +1 -1
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);

Indeed max length of task name is 16 bytes, and "dlm_reco_thread" plus '\0' have taken all the space. So indeed above code is useless. Can we rename this name and maybe other one(like "dlm_thread") to leave more space for domain marker?

Quoted 4 lines

Yes, you are right. For dlm_reco_thread it won't print any uuid bytes. I put it here just for code consistency.
It is really hard for me to rename it to a better one:)
Any suggestions?

Quoted 9 lines

How about this?

dlmwq-xxxx
dlmrec-xxxx
dlm-xxxx
o2dc-xxxx
o2cmt-xxx
o2rec-xxx

Thanks very much for your advice. But from our experience, it is usual that we can distinguish the messages if plus 3 uuid bytes. So rename all of them may not be necessary.
Could we only rename "dlm_reco_thread" to "dlm_reco-xxx"?

Thanks,
Joseph

Quoted 10 lines

Thanks,
Junxiao.

Quoted 4 lines

Thanks,
Joseph

#5 lore raw

PATCH 1/1 ocfs2: add uuid to ocfs2 thread name for problem analysis

Quoted 33 lines · Joseph Qi
Quoted 23 lines · Junxiao Bi
Quoted 2 lines · Joseph Qi

Hi Junxiao,

...

Quoted 15 lines
Quoted 7 lines
Quoted 7 lines
fs/ocfs2/dlm/dlmrecovery.c +0 -0
index a43f9ef..570509e 100644
@@ -205,7 +205,7 @@ int dlm_launch_recovery_thread(struct dlm_ctxt *dlm)
 	mlog(0, "starting dlm recovery thread...\n");
Quoted 8 lines

fs/ocfs2/dlm/dlmrecovery.c int dlm_launch_recovery_thread(struct dlm_ctxt *dlm) · line 208

Quoted 3 lines
fs/ocfs2/dlm/dlmrecovery.c +1 -1
 	dlm->dlm_reco_thread_task = kthread_run(dlm_recovery_thread, dlm,
-						"dlm_reco_thread");
+			"dlm_reco_thread-%s", dlm->name);

Indeed max length of task name is 16 bytes, and "dlm_reco_thread" plus '\0' have taken all the space. So indeed above code is useless. Can we rename this name and maybe other one(like "dlm_thread") to leave more space for domain marker?

Quoted 4 lines

Yes, you are right. For dlm_reco_thread it won't print any uuid bytes. I put it here just for code consistency.
It is really hard for me to rename it to a better one:)
Any suggestions?

Quoted 9 lines

How about this?

dlmwq-xxxx
dlmrec-xxxx
dlm-xxxx
o2dc-xxxx
o2cmt-xxx
o2rec-xxx

Quoted 3 lines

Thanks very much for your advice. But from our experience, it is usual that we can distinguish the messages if plus 3 uuid bytes. So rename all of them may not be necessary.

Just want to name the thread in the simple and clear way. The original name is a little complicated.

Quoted 1 line

Could we only rename "dlm_reco_thread" to "dlm_reco-xxx"?

You can do this since this do fix your issue.

Thanks,
Junxiao.

Quoted 16 lines

Thanks,
Joseph

Quoted 10 lines

Thanks,
Junxiao.

Quoted 4 lines

Thanks,
Joseph