All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH] Add prctl to set sibling thread names
@ 2009-10-21 23:21 john stultz
  2009-10-22  0:28 ` Andi Kleen
  2009-10-22  0:48 ` Arjan van de Ven
  0 siblings, 2 replies; 21+ messages in thread
From: john stultz @ 2009-10-21 23:21 UTC (permalink / raw)
  To: Andi Kleen; +Cc: lkml, Mike Fulton, Sean Foley, Darren Hart


Taking a very raw attempt at this, I scratched out the following simple
implementation. I'd appreciate any review or suggestions for
improvements. I'm not at all certain the passing of the thread pid_t
through the unsigned long is valid, for instance, or if
same_thread_group() is the right check to make sure we only change
siblings and not tid from other processes. So any advice on better
approaches would be great.

thanks
-john

========================

Setting a thread's comm to be something unique is a very useful ability
and is helpful for debugging complicated threaded applications. However
currently the only way to set a thread name is for the thread to name
itself via the PR_SET_NAME prctl.

However, there may be situations where it would be advantageous for a
thread dispatcher to be naming the threads its managing, rather then
having the threads self-describe themselves. This sort of behavior is
available on other systems via the pthread_setname_np() interface.

This patch allows a thread to name its sibling threads via a new
PR_SET_THREAD_NAME prctrl.

Signed-off-by: John Stultz <johnstul@us.ibm.com>

diff --git a/include/linux/prctl.h b/include/linux/prctl.h
index 9311505..2726527 100644
--- a/include/linux/prctl.h
+++ b/include/linux/prctl.h
@@ -52,6 +52,7 @@
 
 #define PR_SET_NAME    15		/* Set process name */
 #define PR_GET_NAME    16		/* Get process name */
+#define PR_SET_THREAD_NAME    17	/* Set sibling thread name */
 
 /* Get/set process endian */
 #define PR_GET_ENDIAN	19
diff --git a/kernel/sys.c b/kernel/sys.c
index 255475d..d25851a 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -1506,6 +1506,27 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 					 sizeof(comm)))
 				return -EFAULT;
 			return 0;
+		case PR_SET_THREAD_NAME:
+		{
+			struct task_struct *tsk;
+			pid_t tid;
+
+			comm[sizeof(me->comm)-1] = 0;
+			tid = (pid_t)arg2;
+			if (strncpy_from_user(comm, (char __user *)arg3,
+					      sizeof(me->comm) - 1) < 0)
+				return -EFAULT;
+
+			tsk = get_pid_task(find_get_pid(tid), PIDTYPE_PID);
+			if (!tsk)
+				return -EINVAL;
+
+			if (!same_thread_group(me, tsk))
+				return -EINVAL;
+
+			set_task_comm(tsk, comm);
+			return 0;
+		}
 		case PR_GET_ENDIAN:
 			error = GET_ENDIAN(me, arg2);
 			break;



^ permalink raw reply related	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-21 23:21 [RFC][PATCH] Add prctl to set sibling thread names john stultz
@ 2009-10-22  0:28 ` Andi Kleen
  2009-10-22  0:42   ` john stultz
  2009-10-22  0:48 ` Arjan van de Ven
  1 sibling, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2009-10-22  0:28 UTC (permalink / raw)
  To: john stultz; +Cc: Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, Oct 21, 2009 at 04:21:37PM -0700, john stultz wrote:
> 
> Taking a very raw attempt at this, I scratched out the following simple
> implementation. I'd appreciate any review or suggestions for
> improvements. I'm not at all certain the passing of the thread pid_t
> through the unsigned long is valid, for instance, or if
> same_thread_group() is the right check to make sure we only change
> siblings and not tid from other processes. So any advice on better
> approaches would be great.

First though that comes to mind is that this should not be in prctl()

-Andi

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:28 ` Andi Kleen
@ 2009-10-22  0:42   ` john stultz
  2009-10-22  0:44     ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: john stultz @ 2009-10-22  0:42 UTC (permalink / raw)
  To: Andi Kleen; +Cc: lkml, Mike Fulton, Sean Foley, Darren Hart

On Thu, 2009-10-22 at 02:28 +0200, Andi Kleen wrote:
> On Wed, Oct 21, 2009 at 04:21:37PM -0700, john stultz wrote:
> > 
> > Taking a very raw attempt at this, I scratched out the following simple
> > implementation. I'd appreciate any review or suggestions for
> > improvements. I'm not at all certain the passing of the thread pid_t
> > through the unsigned long is valid, for instance, or if
> > same_thread_group() is the right check to make sure we only change
> > siblings and not tid from other processes. So any advice on better
> > approaches would be great.
> 
> First though that comes to mind is that this should not be in prctl()

So it deserves a new syscall? Any other thoughts?

thanks
-john


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:42   ` john stultz
@ 2009-10-22  0:44     ` Andi Kleen
  0 siblings, 0 replies; 21+ messages in thread
From: Andi Kleen @ 2009-10-22  0:44 UTC (permalink / raw)
  To: john stultz; +Cc: Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, Oct 21, 2009 at 05:42:15PM -0700, john stultz wrote:
> On Thu, 2009-10-22 at 02:28 +0200, Andi Kleen wrote:
> > On Wed, Oct 21, 2009 at 04:21:37PM -0700, john stultz wrote:
> > > 
> > > Taking a very raw attempt at this, I scratched out the following simple
> > > implementation. I'd appreciate any review or suggestions for
> > > improvements. I'm not at all certain the passing of the thread pid_t
> > > through the unsigned long is valid, for instance, or if
> > > same_thread_group() is the right check to make sure we only change
> > > siblings and not tid from other processes. So any advice on better
> > > approaches would be great.
> > 
> > First though that comes to mind is that this should not be in prctl()
> 
> So it deserves a new syscall? Any other thoughts?

I would probably just put it into /proc/pid and use the normal ptrace
access checks.
-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-21 23:21 [RFC][PATCH] Add prctl to set sibling thread names john stultz
  2009-10-22  0:28 ` Andi Kleen
@ 2009-10-22  0:48 ` Arjan van de Ven
  2009-10-22  0:49   ` Andi Kleen
  2009-10-22  0:52   ` john stultz
  1 sibling, 2 replies; 21+ messages in thread
From: Arjan van de Ven @ 2009-10-22  0:48 UTC (permalink / raw)
  To: john stultz; +Cc: Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, 21 Oct 2009 16:21:37 -0700
john stultz <johnstul@us.ibm.com> wrote:

> 
> Taking a very raw attempt at this, I scratched out the following
> simple implementation. I'd appreciate any review or suggestions for
> improvements. I'm not at all certain the passing of the thread pid_t
> through the unsigned long is valid, for instance, or if
> same_thread_group() is the right check to make sure we only change
> siblings and not tid from other processes. So any advice on better
> approaches would be great.
> 
> +				return -EINVAL;
> +
> +			set_task_comm(tsk, comm);


you're pretty much the first now who touches ->comm from
not-the-thread-itself.... are you sure that is safe?


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:48 ` Arjan van de Ven
@ 2009-10-22  0:49   ` Andi Kleen
  2009-10-22  2:48     ` Arjan van de Ven
  2009-10-22  0:52   ` john stultz
  1 sibling, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2009-10-22  0:49 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: john stultz, Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

> you're pretty much the first now who touches ->comm from
> not-the-thread-itself.... are you sure that is safe?

It's not, there is no locking. On the other hand nothing should crash,
just users might see half rewritten data.
-Andi
-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:48 ` Arjan van de Ven
  2009-10-22  0:49   ` Andi Kleen
@ 2009-10-22  0:52   ` john stultz
  2009-10-22  2:00     ` Andrew Morton
  1 sibling, 1 reply; 21+ messages in thread
From: john stultz @ 2009-10-22  0:52 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, 2009-10-21 at 17:48 -0700, Arjan van de Ven wrote:
> On Wed, 21 Oct 2009 16:21:37 -0700
> john stultz <johnstul@us.ibm.com> wrote:
> 
> > 
> > Taking a very raw attempt at this, I scratched out the following
> > simple implementation. I'd appreciate any review or suggestions for
> > improvements. I'm not at all certain the passing of the thread pid_t
> > through the unsigned long is valid, for instance, or if
> > same_thread_group() is the right check to make sure we only change
> > siblings and not tid from other processes. So any advice on better
> > approaches would be great.
> > 
> > +				return -EINVAL;
> > +
> > +			set_task_comm(tsk, comm);
> 
> 
> you're pretty much the first now who touches ->comm from
> not-the-thread-itself.... are you sure that is safe?

No, I'm not sure at all :)

Thanks for pointing this out. I'll see whats needed in set_task_comm().

thanks again
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:52   ` john stultz
@ 2009-10-22  2:00     ` Andrew Morton
  2009-11-05  2:26       ` KOSAKI Motohiro
  0 siblings, 1 reply; 21+ messages in thread
From: Andrew Morton @ 2009-10-22  2:00 UTC (permalink / raw)
  To: john stultz
  Cc: Arjan van de Ven, Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, 21 Oct 2009 17:52:24 -0700 john stultz <johnstul@us.ibm.com> wrote:

> On Wed, 2009-10-21 at 17:48 -0700, Arjan van de Ven wrote:
> > On Wed, 21 Oct 2009 16:21:37 -0700
> > john stultz <johnstul@us.ibm.com> wrote:
> > 
> > > 
> > > Taking a very raw attempt at this, I scratched out the following
> > > simple implementation. I'd appreciate any review or suggestions for
> > > improvements. I'm not at all certain the passing of the thread pid_t
> > > through the unsigned long is valid, for instance, or if
> > > same_thread_group() is the right check to make sure we only change
> > > siblings and not tid from other processes. So any advice on better
> > > approaches would be great.
> > > 
> > > +				return -EINVAL;
> > > +
> > > +			set_task_comm(tsk, comm);
> > 
> > 
> > you're pretty much the first now who touches ->comm from
> > not-the-thread-itself.... are you sure that is safe?
> 
> No, I'm not sure at all :)
> 
> Thanks for pointing this out. I'll see whats needed in set_task_comm().
> 

set_task_comm() is OK.  The problem will be the unwritten rule that
processes can read *their own* ->comm without task_lock(), because nobody
ever alters ->comm apart from tack which owns it.

You've changed that, so all the open-coded accesses to current->comm are
now racy.

Also, you appear to be running set_task_comm() against a task_struct
without holding a reference on that task.  Will a well-timed exit() cause a
modify-after-free?



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  0:49   ` Andi Kleen
@ 2009-10-22  2:48     ` Arjan van de Ven
  2009-10-24  3:54       ` Andi Kleen
  0 siblings, 1 reply; 21+ messages in thread
From: Arjan van de Ven @ 2009-10-22  2:48 UTC (permalink / raw)
  To: Andi Kleen
  Cc: john stultz, Andi Kleen, lkml, Mike Fulton, Sean Foley, Darren Hart

On Thu, 22 Oct 2009 02:49:19 +0200
Andi Kleen <andi@firstfloor.org> wrote:

> > you're pretty much the first now who touches ->comm from
> > not-the-thread-itself.... are you sure that is safe?
> 
> It's not, there is no locking. On the other hand nothing should crash,
> just users might see half rewritten data.

.. with strings this is tricky though.. if the new string is longer
than the old one the terminating zero might just be missed etc.



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  2:48     ` Arjan van de Ven
@ 2009-10-24  3:54       ` Andi Kleen
  2009-10-26 23:56         ` john stultz
  0 siblings, 1 reply; 21+ messages in thread
From: Andi Kleen @ 2009-10-24  3:54 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Andi Kleen, john stultz, lkml, Mike Fulton, Sean Foley, Darren Hart

On Wed, Oct 21, 2009 at 07:48:22PM -0700, Arjan van de Ven wrote:
> On Thu, 22 Oct 2009 02:49:19 +0200
> Andi Kleen <andi@firstfloor.org> wrote:
> 
> > > you're pretty much the first now who touches ->comm from
> > > not-the-thread-itself.... are you sure that is safe?
> > 
> > It's not, there is no locking. On the other hand nothing should crash,
> > just users might see half rewritten data.
> 
> .. with strings this is tricky though.. if the new string is longer
> than the old one the terminating zero might just be missed etc.

Good point.
He needs to first set the last byte to zero to avoid that.
But better probably to do a proper lock on all readers. Or not add
this feature at all (does it have a strong use case?)
-Andi

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-24  3:54       ` Andi Kleen
@ 2009-10-26 23:56         ` john stultz
  0 siblings, 0 replies; 21+ messages in thread
From: john stultz @ 2009-10-26 23:56 UTC (permalink / raw)
  To: Andi Kleen; +Cc: Arjan van de Ven, lkml, Mike Fulton, Sean Foley, Darren Hart

On Sat, 2009-10-24 at 05:54 +0200, Andi Kleen wrote:
> On Wed, Oct 21, 2009 at 07:48:22PM -0700, Arjan van de Ven wrote:
> > On Thu, 22 Oct 2009 02:49:19 +0200
> > Andi Kleen <andi@firstfloor.org> wrote:
> > 
> > > > you're pretty much the first now who touches ->comm from
> > > > not-the-thread-itself.... are you sure that is safe?
> > > 
> > > It's not, there is no locking. On the other hand nothing should crash,
> > > just users might see half rewritten data.
> > 
> > .. with strings this is tricky though.. if the new string is longer
> > than the old one the terminating zero might just be missed etc.
> 
> Good point.
> He needs to first set the last byte to zero to avoid that.
> But better probably to do a proper lock on all readers. Or not add
> this feature at all (does it have a strong use case?)

Thread naming is really helpful for debugging a large multi-threaded
application. But currently it requires the threads to name themselves. 

In some applications there may be a dispatch thread that spawns off the
siblings, and it has more context for naming the threads then the
threads do themselves.

So in that case, currently in order to have named threads, the
application's dispatch thread assigns names, and the threads have to
occasionally poll to see if they need to name themselves to something.

So they can get it to work, but its ugly and a big pain. Other OSes
support pthread_set_name_np() functionality, which makes things easy for
them, so they've requested to see if Linux can't do something similar.

thanks
-john


^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-10-22  2:00     ` Andrew Morton
@ 2009-11-05  2:26       ` KOSAKI Motohiro
  2009-11-05  5:17         ` Darren Hart
  0 siblings, 1 reply; 21+ messages in thread
From: KOSAKI Motohiro @ 2009-11-05  2:26 UTC (permalink / raw)
  To: Andrew Morton
  Cc: kosaki.motohiro, john stultz, Arjan van de Ven, Andi Kleen, lkml,
	Mike Fulton, Sean Foley, Darren Hart

> On Wed, 21 Oct 2009 17:52:24 -0700 john stultz <johnstul@us.ibm.com> wrote:
> 
> > On Wed, 2009-10-21 at 17:48 -0700, Arjan van de Ven wrote:
> > > On Wed, 21 Oct 2009 16:21:37 -0700
> > > john stultz <johnstul@us.ibm.com> wrote:
> > > 
> > > > 
> > > > Taking a very raw attempt at this, I scratched out the following
> > > > simple implementation. I'd appreciate any review or suggestions for
> > > > improvements. I'm not at all certain the passing of the thread pid_t
> > > > through the unsigned long is valid, for instance, or if
> > > > same_thread_group() is the right check to make sure we only change
> > > > siblings and not tid from other processes. So any advice on better
> > > > approaches would be great.
> > > > 
> > > > +				return -EINVAL;
> > > > +
> > > > +			set_task_comm(tsk, comm);
> > > 
> > > 
> > > you're pretty much the first now who touches ->comm from
> > > not-the-thread-itself.... are you sure that is safe?
> > 
> > No, I'm not sure at all :)
> > 
> > Thanks for pointing this out. I'll see whats needed in set_task_comm().
> > 
> 
> set_task_comm() is OK.  The problem will be the unwritten rule that
> processes can read *their own* ->comm without task_lock(), because nobody
> ever alters ->comm apart from tack which owns it.
> 
> You've changed that, so all the open-coded accesses to current->comm are
> now racy.
> 
> Also, you appear to be running set_task_comm() against a task_struct
> without holding a reference on that task.  Will a well-timed exit() cause a
> modify-after-free?

John, I'd prefer to suggested another design.
How about this?

1. remove pid argument from prctl
2. cancel pthread_setname_np()
3. instead, create pthread_attr_setname_np()
4. pthread_create() change own thread name by pthread_attr.


It avoid many racy problem automatically.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-05  2:26       ` KOSAKI Motohiro
@ 2009-11-05  5:17         ` Darren Hart
  2009-11-05  5:22           ` KOSAKI Motohiro
  0 siblings, 1 reply; 21+ messages in thread
From: Darren Hart @ 2009-11-05  5:17 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Andrew Morton, john stultz, Arjan van de Ven, Andi Kleen, lkml,
	Mike Fulton, Sean Foley

KOSAKI Motohiro wrote:

> John, I'd prefer to suggested another design.
> How about this?
> 
> 1. remove pid argument from prctl
> 2. cancel pthread_setname_np()
> 3. instead, create pthread_attr_setname_np()
> 4. pthread_create() change own thread name by pthread_attr.
> 
> 
> It avoid many racy problem automatically.

Perhaps, but it also greatly reduces the flexibility of the 
implementation by restricting name changes to create time.

-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-05  5:17         ` Darren Hart
@ 2009-11-05  5:22           ` KOSAKI Motohiro
  2009-11-05  5:36             ` Darren Hart
  0 siblings, 1 reply; 21+ messages in thread
From: KOSAKI Motohiro @ 2009-11-05  5:22 UTC (permalink / raw)
  To: Darren Hart
  Cc: kosaki.motohiro, Andrew Morton, john stultz, Arjan van de Ven,
	Andi Kleen, lkml, Mike Fulton, Sean Foley

> KOSAKI Motohiro wrote:
> 
> > John, I'd prefer to suggested another design.
> > How about this?
> > 
> > 1. remove pid argument from prctl
> > 2. cancel pthread_setname_np()
> > 3. instead, create pthread_attr_setname_np()
> > 4. pthread_create() change own thread name by pthread_attr.
> > 
> > It avoid many racy problem automatically.
> 
> Perhaps, but it also greatly reduces the flexibility of the 
> implementation by restricting name changes to create time.

Hm.
if your program really need to change another thread name, can you please tell us
why it is necessary and when it is used?




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-05  5:22           ` KOSAKI Motohiro
@ 2009-11-05  5:36             ` Darren Hart
  2009-11-05  5:42               ` KOSAKI Motohiro
  0 siblings, 1 reply; 21+ messages in thread
From: Darren Hart @ 2009-11-05  5:36 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Andrew Morton, john stultz, Arjan van de Ven, Andi Kleen, lkml,
	Mike Fulton, Sean Foley

KOSAKI Motohiro wrote:
>> KOSAKI Motohiro wrote:
>>
>>> John, I'd prefer to suggested another design.
>>> How about this?
>>>
>>> 1. remove pid argument from prctl
>>> 2. cancel pthread_setname_np()
>>> 3. instead, create pthread_attr_setname_np()
>>> 4. pthread_create() change own thread name by pthread_attr.
>>>
>>> It avoid many racy problem automatically.
>> Perhaps, but it also greatly reduces the flexibility of the 
>> implementation by restricting name changes to create time.
> 
> Hm.
> if your program really need to change another thread name, can you please tell us
> why it is necessary and when it is used?
> 

I think John's previous mails covered that pretty well. As for doing the 
name change at create time, or sometime later, it just seems to me that 
the flexibility of doing so later is worth having. While I know we don't 
have to follow other systems implementations, in this case 
pthread_setname_np() seems a reasonable model to follow to me.

-- 
Darren Hart
IBM Linux Technology Center
Real-Time Linux Team

^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-05  5:36             ` Darren Hart
@ 2009-11-05  5:42               ` KOSAKI Motohiro
  2009-11-05 19:11                 ` john stultz
       [not found]                 ` <OF5EE04242.D2B67AF2-ON85257665.0064683D-85257665.0068209E@ca.ibm.com>
  0 siblings, 2 replies; 21+ messages in thread
From: KOSAKI Motohiro @ 2009-11-05  5:42 UTC (permalink / raw)
  To: Darren Hart
  Cc: kosaki.motohiro, Andrew Morton, john stultz, Arjan van de Ven,
	Andi Kleen, lkml, Mike Fulton, Sean Foley

> KOSAKI Motohiro wrote:
> >> KOSAKI Motohiro wrote:
> >>
> >>> John, I'd prefer to suggested another design.
> >>> How about this?
> >>>
> >>> 1. remove pid argument from prctl
> >>> 2. cancel pthread_setname_np()
> >>> 3. instead, create pthread_attr_setname_np()
> >>> 4. pthread_create() change own thread name by pthread_attr.
> >>>
> >>> It avoid many racy problem automatically.
> >> Perhaps, but it also greatly reduces the flexibility of the 
> >> implementation by restricting name changes to create time.
> > 
> > Hm.
> > if your program really need to change another thread name, can you please tell us
> > why it is necessary and when it is used?
> 
> I think John's previous mails covered that pretty well. As for doing the 
> name change at create time, or sometime later, it just seems to me that 
> the flexibility of doing so later is worth having. While I know we don't 
> have to follow other systems implementations, in this case 
> pthread_setname_np() seems a reasonable model to follow to me.

You only said your mode is more flexible. but I want to know _why_ this flexibiliby is
necessay. please tell us concrete use-case.



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-05  5:42               ` KOSAKI Motohiro
@ 2009-11-05 19:11                 ` john stultz
       [not found]                 ` <OF5EE04242.D2B67AF2-ON85257665.0064683D-85257665.0068209E@ca.ibm.com>
  1 sibling, 0 replies; 21+ messages in thread
From: john stultz @ 2009-11-05 19:11 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Darren Hart, Andrew Morton, Arjan van de Ven, Andi Kleen, lkml,
	Mike Fulton, Sean Foley

On Thu, 2009-11-05 at 14:42 +0900, KOSAKI Motohiro wrote:
> > KOSAKI Motohiro wrote:
> > >> KOSAKI Motohiro wrote:
> > >>
> > >>> John, I'd prefer to suggested another design.
> > >>> How about this?
> > >>>
> > >>> 1. remove pid argument from prctl
> > >>> 2. cancel pthread_setname_np()
> > >>> 3. instead, create pthread_attr_setname_np()
> > >>> 4. pthread_create() change own thread name by pthread_attr.
> > >>>
> > >>> It avoid many racy problem automatically.
> > >> Perhaps, but it also greatly reduces the flexibility of the 
> > >> implementation by restricting name changes to create time.
> > > 
> > > Hm.
> > > if your program really need to change another thread name, can you please tell us
> > > why it is necessary and when it is used?
> > 
> > I think John's previous mails covered that pretty well. As for doing the 
> > name change at create time, or sometime later, it just seems to me that 
> > the flexibility of doing so later is worth having. While I know we don't 
> > have to follow other systems implementations, in this case 
> > pthread_setname_np() seems a reasonable model to follow to me.
> 
> You only said your mode is more flexible. but I want to know _why_ this flexibiliby is
> necessay. please tell us concrete use-case.

You can read Sean's example from this thread here:
	http://lkml.org/lkml/2009/10/27/259

thanks
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
       [not found]                 ` <OF5EE04242.D2B67AF2-ON85257665.0064683D-85257665.0068209E@ca.ibm.com>
@ 2009-11-10  5:27                   ` KOSAKI Motohiro
  2009-11-10 20:16                     ` john stultz
  0 siblings, 1 reply; 21+ messages in thread
From: KOSAKI Motohiro @ 2009-11-10  5:27 UTC (permalink / raw)
  To: Sean Foley, john stultz
  Cc: kosaki.motohiro, Andrew Morton, Andi Kleen, Arjan van de Ven,
	Darren Hart, lkml, Mike Fulton

Hi Sean, John,

> Kosaki,
> Here are a couple of use cases previously posted to this thread on the linux kernel mailing list:
> 
> dispatch thread adds context to thread names:
> http://marc.info/?l=linux-kernel&m=125660141231348&w=2
> 
> java language support:
> http://marc.info/?l=linux-kernel&m=125666430720863&w=2
> 
> 
> 
> Here are some various specific use cases from the web:
> 
> Attaching additional info to thread names when used for different purposes:
> http://osdir.com/ml/java.jsr.166-concurrency/2006-12/msg00105.html
> 
> Threads obtained from thread pools being reassigned new names:
> http://haacked.com/archive/2004/06/07/546.aspx
> http://bytes.com/topic/c-sharp/answers/637152-naming-backgroundworker-thread
> 
> Renaming threads scattered across third-party libraries by enumerating them and renaming them dynamically:
> http://stackoverflow.com/questions/467224/renaming-threads-in-java

Okey, good explanation. thanks!

So, I would suggested to extend /proc/{pid}/cmdline instead using task->comm.
because 
  - task->comm has nasty locking rule. It is harder to change SMP safe.
  - ps (and other procps tools) already support /proc/{pid}/cmdline.
  - task->comm is restrected 16 character length, /proc/cmdline isn't.

You can see prctl-add-pr_set_proctitle_area-option.patch in -mm tree
as enhancement example at first step.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-10  5:27                   ` KOSAKI Motohiro
@ 2009-11-10 20:16                     ` john stultz
  2009-11-11  0:04                       ` KOSAKI Motohiro
  0 siblings, 1 reply; 21+ messages in thread
From: john stultz @ 2009-11-10 20:16 UTC (permalink / raw)
  To: KOSAKI Motohiro
  Cc: Sean Foley, Andrew Morton, Andi Kleen, Arjan van de Ven,
	Darren Hart, lkml, Mike Fulton

On Tue, 2009-11-10 at 14:27 +0900, KOSAKI Motohiro wrote:
> Hi Sean, John,
> 
> > Kosaki,
> > Here are a couple of use cases previously posted to this thread on the linux kernel mailing list:
> > 
> > dispatch thread adds context to thread names:
> > http://marc.info/?l=linux-kernel&m=125660141231348&w=2
> > 
> > java language support:
> > http://marc.info/?l=linux-kernel&m=125666430720863&w=2
> > 
> > 
> > 
> > Here are some various specific use cases from the web:
> > 
> > Attaching additional info to thread names when used for different purposes:
> > http://osdir.com/ml/java.jsr.166-concurrency/2006-12/msg00105.html
> > 
> > Threads obtained from thread pools being reassigned new names:
> > http://haacked.com/archive/2004/06/07/546.aspx
> > http://bytes.com/topic/c-sharp/answers/637152-naming-backgroundworker-thread
> > 
> > Renaming threads scattered across third-party libraries by enumerating them and renaming them dynamically:
> > http://stackoverflow.com/questions/467224/renaming-threads-in-java
> 
> Okey, good explanation. thanks!
> 
> So, I would suggested to extend /proc/{pid}/cmdline instead using task->comm.
> because 
>   - task->comm has nasty locking rule. It is harder to change SMP safe.

I cc'ed you on the updated patch which addresses this. Please let me
know if you have specific concerns there.

>   - ps (and other procps tools) already support /proc/{pid}/cmdline.
>   - task->comm is restrected 16 character length, /proc/cmdline isn't.

Part of the reason to use comm is that most tools like perf or oprofile,
use comm, instead of cmd. Additionally, looking at cmdline, that's per
mm not per task, right? So it wouldn't really work for thread names. 

Please correct me if I'm not seeing what you're really suggesting.

thanks
-john



^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
  2009-11-10 20:16                     ` john stultz
@ 2009-11-11  0:04                       ` KOSAKI Motohiro
  0 siblings, 0 replies; 21+ messages in thread
From: KOSAKI Motohiro @ 2009-11-11  0:04 UTC (permalink / raw)
  To: john stultz
  Cc: kosaki.motohiro, Sean Foley, Andrew Morton, Andi Kleen,
	Arjan van de Ven, Darren Hart, lkml, Mike Fulton

> On Tue, 2009-11-10 at 14:27 +0900, KOSAKI Motohiro wrote:
> > Hi Sean, John,
> > 
> > > Kosaki,
> > > Here are a couple of use cases previously posted to this thread on the linux kernel mailing list:
> > > 
> > > dispatch thread adds context to thread names:
> > > http://marc.info/?l=linux-kernel&m=125660141231348&w=2
> > > 
> > > java language support:
> > > http://marc.info/?l=linux-kernel&m=125666430720863&w=2
> > > 
> > > 
> > > 
> > > Here are some various specific use cases from the web:
> > > 
> > > Attaching additional info to thread names when used for different purposes:
> > > http://osdir.com/ml/java.jsr.166-concurrency/2006-12/msg00105.html
> > > 
> > > Threads obtained from thread pools being reassigned new names:
> > > http://haacked.com/archive/2004/06/07/546.aspx
> > > http://bytes.com/topic/c-sharp/answers/637152-naming-backgroundworker-thread
> > > 
> > > Renaming threads scattered across third-party libraries by enumerating them and renaming them dynamically:
> > > http://stackoverflow.com/questions/467224/renaming-threads-in-java
> > 
> > Okey, good explanation. thanks!
> > 
> > So, I would suggested to extend /proc/{pid}/cmdline instead using task->comm.
> > because 
> >   - task->comm has nasty locking rule. It is harder to change SMP safe.
> 
> I cc'ed you on the updated patch which addresses this. Please let me
> know if you have specific concerns there.
> 
> >   - ps (and other procps tools) already support /proc/{pid}/cmdline.
> >   - task->comm is restrected 16 character length, /proc/cmdline isn't.
> 
> Part of the reason to use comm is that most tools like perf or oprofile,
> use comm, instead of cmd. 

Ah, good reason. Okey, I will revew your new patch.

thanks.


> Additionally, looking at cmdline, that's per
> mm not per task, right? So it wouldn't really work for thread names. 

Currently, yes. I meaned I think you can enhance it ;)

> Please correct me if I'm not seeing what you're really suggesting.




^ permalink raw reply	[flat|nested] 21+ messages in thread

* Re: [RFC][PATCH] Add prctl to set sibling thread names
@ 2009-11-05 19:03 Sean Foley
  0 siblings, 0 replies; 21+ messages in thread
From: Sean Foley @ 2009-11-05 19:03 UTC (permalink / raw)
  To: linux-kernel

Sean Foley
J9 Real-Time Java Ottawa Technical Lead
Sean_Foley@ca.ibm.com (613)356-5012

KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com> wrote on 11/05/2009 
12:42:23 AM:

> > KOSAKI Motohiro wrote:
> > >> KOSAKI Motohiro wrote:
> > >>
> > >>> John, I'd prefer to suggested another design.
> > >>> How about this?
> > >>>
> > >>> 1. remove pid argument from prctl
> > >>> 2. cancel pthread_setname_np()
> > >>> 3. instead, create pthread_attr_setname_np()
> > >>> 4. pthread_create() change own thread name by pthread_attr.
> > >>>
> > >>> It avoid many racy problem automatically.
> > >> Perhaps, but it also greatly reduces the flexibility of the 
> > >> implementation by restricting name changes to create time.
> > > 
> > > Hm.
> > > if your program really need to change another thread name, can 
> you please tell us
> > > why it is necessary and when it is used?
> > 
> > I think John's previous mails covered that pretty well. As for doing 
the 
> > name change at create time, or sometime later, it just seems to me 
that 
> > the flexibility of doing so later is worth having. While I know we 
don't 
> > have to follow other systems implementations, in this case 
> > pthread_setname_np() seems a reasonable model to follow to me.
> 
> You only said your mode is more flexible. but I want to know _why_ 
> this flexibiliby is
> necessay. please tell us concrete use-case.
> 

Kosaki,
Here are a couple of use cases previously posted to this thread on the 
linux kernel mailing list:

dispatch thread adds context to thread names:
http://marc.info/?l=linux-kernel&m=125660141231348&w=2

java language support:
http://marc.info/?l=linux-kernel&m=125666430720863&w=2



Here are some various specific use cases from the web:

Attaching additional info to thread names when used for different 
purposes:
http://osdir.com/ml/java.jsr.166-concurrency/2006-12/msg00105.html

Threads obtained from thread pools being reassigned new names:
http://haacked.com/archive/2004/06/07/546.aspx
http://bytes.com/topic/c-sharp/answers/637152-naming-backgroundworker-thread

Renaming threads scattered across third-party libraries by enumerating 
them and renaming them dynamically:
http://stackoverflow.com/questions/467224/renaming-threads-in-java



^ permalink raw reply	[flat|nested] 21+ messages in thread

end of thread, other threads:[~2009-11-11  0:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-10-21 23:21 [RFC][PATCH] Add prctl to set sibling thread names john stultz
2009-10-22  0:28 ` Andi Kleen
2009-10-22  0:42   ` john stultz
2009-10-22  0:44     ` Andi Kleen
2009-10-22  0:48 ` Arjan van de Ven
2009-10-22  0:49   ` Andi Kleen
2009-10-22  2:48     ` Arjan van de Ven
2009-10-24  3:54       ` Andi Kleen
2009-10-26 23:56         ` john stultz
2009-10-22  0:52   ` john stultz
2009-10-22  2:00     ` Andrew Morton
2009-11-05  2:26       ` KOSAKI Motohiro
2009-11-05  5:17         ` Darren Hart
2009-11-05  5:22           ` KOSAKI Motohiro
2009-11-05  5:36             ` Darren Hart
2009-11-05  5:42               ` KOSAKI Motohiro
2009-11-05 19:11                 ` john stultz
     [not found]                 ` <OF5EE04242.D2B67AF2-ON85257665.0064683D-85257665.0068209E@ca.ibm.com>
2009-11-10  5:27                   ` KOSAKI Motohiro
2009-11-10 20:16                     ` john stultz
2009-11-11  0:04                       ` KOSAKI Motohiro
2009-11-05 19:03 Sean Foley

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.