[Ocfs2-devel] [PATCH] CMWQ: Set workqueue name before we process one work.

[Ocfs2-devel] [PATCH] CMWQ: Set workqueue name before we process one work.

[Tao Ma]

Now in CMWQ, workqueue threads are named as 'kworker/*'. So it is
a little boring to see in the 'top'(below) and actually it isn't
meaningful for the users.
12606 root      20   0     0    0    0 S    2  0.0   0:03.22 kworker/u:0
12607 root      20   0     0    0    0 S    1  0.0   0:03.69 kworker/u:4

So this patch just try to set the proper workqueue name if it does
exist. Yes, workqueue now is a thread pool and the data may be not
accurate but I think it is better(below) than the 'kworker*' stuff.
And if there is something block the workqueue, we can find the caller
easily.
12606 root      20   0     0    0    0 D    2  0.0   0:02.90 dlm_wq
12607 root      20   0     0    0    0 D    0  0.0   0:03.21 ocfs2_wq

What's more, in ocfs2, we sometimes want to print some debug info in
the system log and we use 'current->comm' to print the thread and this
change also does help.

The only thing I am not clear is that do we need [gs]et_task_comm?
I guess not and this patch just try to use strlcpy without task_lock.

Cc: Tejun Heo <tj@kernel.org>
Signed-off-by: Tao Ma <tao.ma@oracle.com>
---
 kernel/workqueue.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/kernel/workqueue.c b/kernel/workqueue.c
index 2994a0e..69402f8 100644
--- a/kernel/workqueue.c
+++ b/kernel/workqueue.c
@@ -1730,6 +1730,7 @@ static void process_one_work(struct worker *worker, struct work_struct *work)
 	work_func_t f = work->func;
 	int work_color;
 	struct worker *collision;
+	char comm_saved[TASK_COMM_LEN];
 #ifdef CONFIG_LOCKDEP
 	/*
 	 * It is permissible to free the struct work_struct from
@@ -1790,7 +1791,16 @@ static void process_one_work(struct worker *worker, struct work_struct *work)
 	work_clear_pending(work);
 	lock_map_acquire(&cwq->wq->lockdep_map);
 	lock_map_acquire(&lockdep_map);
+	if (cwq->wq->name) {
+		strlcpy(comm_saved, worker->task->comm,
+			sizeof(comm_saved));
+		strlcpy(worker->task->comm, cwq->wq->name,
+			sizeof(worker->task->comm));
+	}
 	f(work);
+	if (cwq->wq->name)
+		strlcpy(worker->task->comm, comm_saved,
+			sizeof(worker->task->comm));
 	lock_map_release(&lockdep_map);
 	lock_map_release(&cwq->wq->lockdep_map);
 
-- 
1.7.1.GIT

[Ocfs2-devel] [PATCH] CMWQ: Set workqueue name before we process one work.

[Tejun Heo]

On 08/16/2010 09:39 AM, Tao Ma wrote:
> Now in CMWQ, workqueue threads are named as 'kworker/*'. So it is
> a little boring to see in the 'top'(below) and actually it isn't
> meaningful for the users.
> 12606 root      20   0     0    0    0 S    2  0.0   0:03.22 kworker/u:0
> 12607 root      20   0     0    0    0 S    1  0.0   0:03.69 kworker/u:4
> 
> So this patch just try to set the proper workqueue name if it does
> exist. Yes, workqueue now is a thread pool and the data may be not
> accurate but I think it is better(below) than the 'kworker*' stuff.
> And if there is something block the workqueue, we can find the caller
> easily.
> 12606 root      20   0     0    0    0 D    2  0.0   0:02.90 dlm_wq
> 12607 root      20   0     0    0    0 D    0  0.0   0:03.21 ocfs2_wq
> 
> What's more, in ocfs2, we sometimes want to print some debug info in
> the system log and we use 'current->comm' to print the thread and this
> change also does help.
> 
> The only thing I am not clear is that do we need [gs]et_task_comm?
> I guess not and this patch just try to use strlcpy without task_lock.
> 
> Cc: Tejun Heo <tj@kernel.org>
> Signed-off-by: Tao Ma <tao.ma@oracle.com>

I thought about the same thing but this doesn't provide any more
information than stack traces for debugging and for run time tracking
implementing tracing API (scheduled to be added in this devel cycle)
is the right approach.  Also, I don't think it's wise to constantly
change program name.  It will confuse more than help.  So, unless
there are other reasons for doing this, I don't think I'm gonna take
this one.

Thanks.

-- 
tejun

[Ocfs2-devel] [PATCH] CMWQ: Set workqueue name before we process one work.

[Tao Ma]

Hi Tejun,
	Thanks for the quick response.
On 08/16/2010 03:44 PM, Tejun Heo wrote:
> On 08/16/2010 09:39 AM, Tao Ma wrote:
>> Now in CMWQ, workqueue threads are named as 'kworker/*'. So it is
>> a little boring to see in the 'top'(below) and actually it isn't
>> meaningful for the users.
>> 12606 root      20   0     0    0    0 S    2  0.0   0:03.22 kworker/u:0
>> 12607 root      20   0     0    0    0 S    1  0.0   0:03.69 kworker/u:4
>>
>> So this patch just try to set the proper workqueue name if it does
>> exist. Yes, workqueue now is a thread pool and the data may be not
>> accurate but I think it is better(below) than the 'kworker*' stuff.
>> And if there is something block the workqueue, we can find the caller
>> easily.
>> 12606 root      20   0     0    0    0 D    2  0.0   0:02.90 dlm_wq
>> 12607 root      20   0     0    0    0 D    0  0.0   0:03.21 ocfs2_wq
>>
>> What's more, in ocfs2, we sometimes want to print some debug info in
>> the system log and we use 'current->comm' to print the thread and this
>> change also does help.
>>
>> The only thing I am not clear is that do we need [gs]et_task_comm?
>> I guess not and this patch just try to use strlcpy without task_lock.
>>
>> Cc: Tejun Heo<tj@kernel.org>
>> Signed-off-by: Tao Ma<tao.ma@oracle.com>
>
> I thought about the same thing but this doesn't provide any more
> information than stack traces for debugging and for run time tracking
> implementing tracing API (scheduled to be added in this devel cycle)
> is the right approach.
Do you have the link for this or it will show up in the mail list soon?
> change program name.  It will confuse more than help.  So, unless
> there are other reasons for doing this, I don't think I'm gonna take
> this one.
OK, let me find other ways ocfs2 can use to trace the real callers of 
some functions in runtime.

Regards,
Tao

[Ocfs2-devel] [PATCH] CMWQ: Set workqueue name before we process one work.

[Tejun Heo]

Hello,

On 08/16/2010 10:03 AM, Tao Ma wrote:
>> I thought about the same thing but this doesn't provide any more
>> information than stack traces for debugging and for run time tracking
>> implementing tracing API (scheduled to be added in this devel cycle)
>> is the right approach.
>
> Do you have the link for this or it will show up in the mail list soon?

Oh, it's just on top of cmwq todo list.  fscache needs it too so it
will definitely happen sooner than later.  I'll try to remember to cc
eyou when I post patches.

Thanks.

-- 
tejun