[PATCH 1/4] export kernel call get_task_comm().

[PATCH 1/4] export kernel call get_task_comm().

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows drivers who call this function to be compiled modularly.
Otherwise, a driver who is interested in this type of functionality
has to implement their own get_task_comm() call, causing code
duplication in the Linux source tree.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 fs/exec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8328beb..e1ac338 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1004,6 +1004,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
 	task_unlock(tsk);
 	return buf;
 }
+EXPORT_SYMBOL_GPL(get_task_comm);
 
 void set_task_comm(struct task_struct *tsk, char *buf)
 {
-- 
1.7.2.3

Re: [PATCH 1/4] export kernel call get_task_comm().

[David Rientjes]

On Tue, 19 Apr 2011, james_p_freyensee@linux.intel.com wrote:

> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own get_task_comm() call, causing code
> duplication in the Linux source tree.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>

Acked-by: David Rientjes <rientjes@google.com>

There've been some other patchsets proposed recently that need to print a 
thread's comm and since /proc/pid/comm can change the comm's of other 
threads out from under them, it's necessary to serialize access to it with 
task_lock().  This patch certainly makes it easier for modules to do so 
correctly, thanks!

Re: [PATCH 1/4] export kernel call get_task_comm().

[J Freyensee]

On Tue, 2011-04-19 at 16:22 -0700, David Rientjes wrote:
> On Tue, 19 Apr 2011, james_p_freyensee@linux.intel.com wrote:
> 
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This allows drivers who call this function to be compiled modularly.
> > Otherwise, a driver who is interested in this type of functionality
> > has to implement their own get_task_comm() call, causing code
> > duplication in the Linux source tree.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> Acked-by: David Rientjes <rientjes@google.com>
> 
> There've been some other patchsets proposed recently that need to print a 
> thread's comm and since /proc/pid/comm can change the comm's of other 
> threads out from under them, it's necessary to serialize access to it with 
> task_lock().  This patch certainly makes it easier for modules to do so 
> correctly, thanks!

Thanks for the compliment; it's appreciated.  I've made my patch sets
independent from one another.  And I believe a common consensus (at
least with Greg KH, Alan C., Arjan VdV and myself) was achieved that
this is a good patch to apply? So maybe this can be applied to the
kernel, independent of my other patches, so this will fit your needs?

Re: [PATCH 1/4] export kernel call get_task_comm().

[David Rientjes]

On Tue, 19 Apr 2011, J Freyensee wrote:

> > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > This allows drivers who call this function to be compiled modularly.
> > > Otherwise, a driver who is interested in this type of functionality
> > > has to implement their own get_task_comm() call, causing code
> > > duplication in the Linux source tree.
> > > 
> > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > Acked-by: David Rientjes <rientjes@google.com>
> > 
> > There've been some other patchsets proposed recently that need to print a 
> > thread's comm and since /proc/pid/comm can change the comm's of other 
> > threads out from under them, it's necessary to serialize access to it with 
> > task_lock().  This patch certainly makes it easier for modules to do so 
> > correctly, thanks!
> 
> Thanks for the compliment; it's appreciated.  I've made my patch sets
> independent from one another.  And I believe a common consensus (at
> least with Greg KH, Alan C., Arjan VdV and myself) was achieved that
> this is a good patch to apply? So maybe this can be applied to the
> kernel, independent of my other patches, so this will fit your needs?
> 

This patch in particular can probably go through the -mm tree; the last 
activity for get_task_comm() was three years ago (59714d65dfbc 
"get_task_comm(): return the result") which also went through the -mm tree 
and its export is a general service to module authors.  Let's cc Andrew 
and see if he'll merge it.

The only other dependency on this is in your third patch in the series 
which actually has potential stack overflow that I'll comment on there, so 
there's nothing else that has a dependency on this patch being merged 
immediately.

Re: [PATCH 1/4] export kernel call get_task_comm().

[Greg KH]

On Tue, Apr 19, 2011 at 06:22:14PM -0700, David Rientjes wrote:
> On Tue, 19 Apr 2011, J Freyensee wrote:
> 
> > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > 
> > > > This allows drivers who call this function to be compiled modularly.
> > > > Otherwise, a driver who is interested in this type of functionality
> > > > has to implement their own get_task_comm() call, causing code
> > > > duplication in the Linux source tree.
> > > > 
> > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > 
> > > Acked-by: David Rientjes <rientjes@google.com>
> > > 
> > > There've been some other patchsets proposed recently that need to print a 
> > > thread's comm and since /proc/pid/comm can change the comm's of other 
> > > threads out from under them, it's necessary to serialize access to it with 
> > > task_lock().  This patch certainly makes it easier for modules to do so 
> > > correctly, thanks!
> > 
> > Thanks for the compliment; it's appreciated.  I've made my patch sets
> > independent from one another.  And I believe a common consensus (at
> > least with Greg KH, Alan C., Arjan VdV and myself) was achieved that
> > this is a good patch to apply? So maybe this can be applied to the
> > kernel, independent of my other patches, so this will fit your needs?
> > 
> 
> This patch in particular can probably go through the -mm tree; the last 
> activity for get_task_comm() was three years ago (59714d65dfbc 
> "get_task_comm(): return the result") which also went through the -mm tree 
> and its export is a general service to module authors.  Let's cc Andrew 
> and see if he'll merge it.

No, let's wait for the other patches to get acceptable before we merge
it.  We don't need to add exports when there are no users of it in the
tree. It's not for .39 at this point anyway, so no rush at the moment.

thanks,

greg k-h

Re: [PATCH 1/4] export kernel call get_task_comm().

[J Freyensee]

On Tue, 2011-04-19 at 18:43 -0700, Greg KH wrote:
> On Tue, Apr 19, 2011 at 06:22:14PM -0700, David Rientjes wrote:
> > On Tue, 19 Apr 2011, J Freyensee wrote:
> > 
> > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > 
> > > > > This allows drivers who call this function to be compiled modularly.
> > > > > Otherwise, a driver who is interested in this type of functionality
> > > > > has to implement their own get_task_comm() call, causing code
> > > > > duplication in the Linux source tree.
> > > > > 
> > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > 
> > > > Acked-by: David Rientjes <rientjes@google.com>
> > > > 
> > > > There've been some other patchsets proposed recently that need to print a 
> > > > thread's comm and since /proc/pid/comm can change the comm's of other 
> > > > threads out from under them, it's necessary to serialize access to it with 
> > > > task_lock().  This patch certainly makes it easier for modules to do so 
> > > > correctly, thanks!
> > > 
> > > Thanks for the compliment; it's appreciated.  I've made my patch sets
> > > independent from one another.  And I believe a common consensus (at
> > > least with Greg KH, Alan C., Arjan VdV and myself) was achieved that
> > > this is a good patch to apply? So maybe this can be applied to the
> > > kernel, independent of my other patches, so this will fit your needs?
> > > 
> > 
> > This patch in particular can probably go through the -mm tree; the last 
> > activity for get_task_comm() was three years ago (59714d65dfbc 
> > "get_task_comm(): return the result") which also went through the -mm tree 
> > and its export is a general service to module authors.  Let's cc Andrew 
> > and see if he'll merge it.
> 
> No, let's wait for the other patches to get acceptable before we merge
> it.  We don't need to add exports when there are no users of it in the
> tree. It's not for .39 at this point anyway, so no rush at the moment.
> 

But as David pointed out, if there is work other than mine coming that
has already been proposed, wouldn't it be good to get this patch in
place now to start the encouragement of future eyes to just call this
function than re-invent the wheel?

> thanks,
> 
> greg k-h

Re: [PATCH 1/4] export kernel call get_task_comm().

[Greg KH]

On Wed, Apr 20, 2011 at 11:11:23AM -0700, J Freyensee wrote:
> On Tue, 2011-04-19 at 18:43 -0700, Greg KH wrote:
> > On Tue, Apr 19, 2011 at 06:22:14PM -0700, David Rientjes wrote:
> > > On Tue, 19 Apr 2011, J Freyensee wrote:
> > > 
> > > > > > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > > 
> > > > > > This allows drivers who call this function to be compiled modularly.
> > > > > > Otherwise, a driver who is interested in this type of functionality
> > > > > > has to implement their own get_task_comm() call, causing code
> > > > > > duplication in the Linux source tree.
> > > > > > 
> > > > > > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> > > > > 
> > > > > Acked-by: David Rientjes <rientjes@google.com>
> > > > > 
> > > > > There've been some other patchsets proposed recently that need to print a 
> > > > > thread's comm and since /proc/pid/comm can change the comm's of other 
> > > > > threads out from under them, it's necessary to serialize access to it with 
> > > > > task_lock().  This patch certainly makes it easier for modules to do so 
> > > > > correctly, thanks!
> > > > 
> > > > Thanks for the compliment; it's appreciated.  I've made my patch sets
> > > > independent from one another.  And I believe a common consensus (at
> > > > least with Greg KH, Alan C., Arjan VdV and myself) was achieved that
> > > > this is a good patch to apply? So maybe this can be applied to the
> > > > kernel, independent of my other patches, so this will fit your needs?
> > > > 
> > > 
> > > This patch in particular can probably go through the -mm tree; the last 
> > > activity for get_task_comm() was three years ago (59714d65dfbc 
> > > "get_task_comm(): return the result") which also went through the -mm tree 
> > > and its export is a general service to module authors.  Let's cc Andrew 
> > > and see if he'll merge it.
> > 
> > No, let's wait for the other patches to get acceptable before we merge
> > it.  We don't need to add exports when there are no users of it in the
> > tree. It's not for .39 at this point anyway, so no rush at the moment.
> > 
> 
> But as David pointed out, if there is work other than mine coming that
> has already been proposed, wouldn't it be good to get this patch in
> place now to start the encouragement of future eyes to just call this
> function than re-invent the wheel?

No, then those projects submit the patch to export this, if they happen
to get to mainline before this one does.

Again, don't export something unless you are using it at the time, no
"this is to be used hopefully by something in the future" type stuff
please.  Who knows if those future plans ever pan out.

thanks,

greg k-h

Re: [PATCH 1/4] export kernel call get_task_comm().

[Andrew Morton]

On Wed, 20 Apr 2011 12:14:21 -0700
Greg KH <gregkh@suse.de> wrote:

> > But as David pointed out, if there is work other than mine coming that
> > has already been proposed, wouldn't it be good to get this patch in
> > place now to start the encouragement of future eyes to just call this
> > function than re-invent the wheel?
> 
> No, then those projects submit the patch to export this, if they happen
> to get to mainline before this one does.
> 
> Again, don't export something unless you are using it at the time, no
> "this is to be used hopefully by something in the future" type stuff
> please.  Who knows if those future plans ever pan out.

Well there's an easy solution here.  Send the patch to export
get_task_comm(), then send along some patches which fix

z:/usr/src/linux-2.6.39-rc4> grep -r 'current->comm' drivers | wc -l
89

Re: [PATCH 1/4] export kernel call get_task_comm().

[J Freyensee]

On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> On Wed, 20 Apr 2011 12:14:21 -0700
> Greg KH <gregkh@suse.de> wrote:
> 
> > > But as David pointed out, if there is work other than mine coming that
> > > has already been proposed, wouldn't it be good to get this patch in
> > > place now to start the encouragement of future eyes to just call this
> > > function than re-invent the wheel?
> > 
> > No, then those projects submit the patch to export this, if they happen
> > to get to mainline before this one does.
> > 
> > Again, don't export something unless you are using it at the time, no
> > "this is to be used hopefully by something in the future" type stuff
> > please.  Who knows if those future plans ever pan out.
> 
> Well there's an easy solution here.  Send the patch to export
> get_task_comm(), then send along some patches which fix
> 
> z:/usr/src/linux-2.6.39-rc4> grep -r 'current->comm' drivers | wc -l
> 89
> 

Andrew, so does this mean you want me to send this patch to export
get_task_comm() to you then?

Thanks,
Jay

Re: [PATCH 1/4] export kernel call get_task_comm().

[David Rientjes]

On Wed, 20 Apr 2011, Andrew Morton wrote:

> > > But as David pointed out, if there is work other than mine coming that
> > > has already been proposed, wouldn't it be good to get this patch in
> > > place now to start the encouragement of future eyes to just call this
> > > function than re-invent the wheel?
> > 
> > No, then those projects submit the patch to export this, if they happen
> > to get to mainline before this one does.
> > 
> > Again, don't export something unless you are using it at the time, no
> > "this is to be used hopefully by something in the future" type stuff
> > please.  Who knows if those future plans ever pan out.
> 
> Well there's an easy solution here.  Send the patch to export
> get_task_comm(), then send along some patches which fix
> 
> z:/usr/src/linux-2.6.39-rc4> grep -r 'current->comm' drivers | wc -l
> 89
> 

If we go this route and continue to support /proc/pid/comm, then we're 
going to need to ensure nothing that dereferences p->comm for any thread p 
is in a blockable context and that it's acceptable for get_task_comm(p) to 
take task_lock(p), and that requires not nesting it with 
write_lock(&tasklist_lock).

I'm thinking that we're going to want to choose something else other than 
p->alloc_lock to protect p->comm since that lock is protecting so many 
different members of struct task_struct that we may not know whether it's 
held when we want to print p->comm.

Re: [PATCH 1/4] export kernel call get_task_comm().

[Andrew Morton]

On Wed, 20 Apr 2011 16:16:08 -0700
J Freyensee <james_p_freyensee@linux.intel.com> wrote:

> On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> > On Wed, 20 Apr 2011 12:14:21 -0700
> > Greg KH <gregkh@suse.de> wrote:
> > 
> > > > But as David pointed out, if there is work other than mine coming that
> > > > has already been proposed, wouldn't it be good to get this patch in
> > > > place now to start the encouragement of future eyes to just call this
> > > > function than re-invent the wheel?
> > > 
> > > No, then those projects submit the patch to export this, if they happen
> > > to get to mainline before this one does.
> > > 
> > > Again, don't export something unless you are using it at the time, no
> > > "this is to be used hopefully by something in the future" type stuff
> > > please.  Who knows if those future plans ever pan out.
> > 
> > Well there's an easy solution here.  Send the patch to export
> > get_task_comm(), then send along some patches which fix
> > 
> > z:/usr/src/linux-2.6.39-rc4> grep -r 'current->comm' drivers | wc -l
> > 89
> > 
> 
> Andrew, so does this mean you want me to send this patch to export
> get_task_comm() to you then?
> 

That works.  But Greg might see us doing it, so some additional
mergeable patches which *need* that export will keep him happy.

(iow, you're being extorted into doing some kernel cleanup work)

Re: [PATCH 1/4] export kernel call get_task_comm().

[J Freyensee]

On Wed, 2011-04-20 at 16:46 -0700, Andrew Morton wrote:
> On Wed, 20 Apr 2011 16:16:08 -0700
> J Freyensee <james_p_freyensee@linux.intel.com> wrote:
> 
> > On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> > > On Wed, 20 Apr 2011 12:14:21 -0700
> > > Greg KH <gregkh@suse.de> wrote:
> > > 
> > > > > But as David pointed out, if there is work other than mine coming that
> > > > > has already been proposed, wouldn't it be good to get this patch in
> > > > > place now to start the encouragement of future eyes to just call this
> > > > > function than re-invent the wheel?
> > > > 
> > > > No, then those projects submit the patch to export this, if they happen
> > > > to get to mainline before this one does.
> > > > 
> > > > Again, don't export something unless you are using it at the time, no
> > > > "this is to be used hopefully by something in the future" type stuff
> > > > please.  Who knows if those future plans ever pan out.
> > > 
> > > Well there's an easy solution here.  Send the patch to export
> > > get_task_comm(), then send along some patches which fix
> > > 
> > > z:/usr/src/linux-2.6.39-rc4> grep -r 'current->comm' drivers | wc -l
> > > 89
> > > 
> > 
> > Andrew, so does this mean you want me to send this patch to export
> > get_task_comm() to you then?
> > 
> 
> That works.  But Greg might see us doing it, so some additional
> mergeable patches which *need* that export will keep him happy.
> 

Okay, so the pti driver I own needs that export to be able to build as a
module, so that is a start :-)

> (iow, you're being extorted into doing some kernel cleanup work)

[PATCH 1/4] export kernel call get_task_comm().

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows drivers who call this function to be compiled modularly.
Otherwise, a driver who is interested in this type of functionality
has to implement their own get_task_comm() call, causing code
duplication in the Linux source tree.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
---
 fs/exec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8328beb..e1ac338 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1004,6 +1004,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
 	task_unlock(tsk);
 	return buf;
 }
+EXPORT_SYMBOL_GPL(get_task_comm);
 
 void set_task_comm(struct task_struct *tsk, char *buf)
 {
-- 
1.7.2.3

Re: [PATCH 1/4] export kernel call get_task_comm().

[David Rientjes]

On Fri, 22 Apr 2011, james_p_freyensee@linux.intel.com wrote:

> From: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> This allows drivers who call this function to be compiled modularly.
> Otherwise, a driver who is interested in this type of functionality
> has to implement their own get_task_comm() call, causing code
> duplication in the Linux source tree.
> 
> Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>

Third time:

Acked-by: David Rientjes <rientjes@google.com>

Re: [PATCH 1/4] export kernel call get_task_comm().

[J Freyensee]

On Mon, 2011-04-25 at 13:57 -0700, David Rientjes wrote:
> On Fri, 22 Apr 2011, james_p_freyensee@linux.intel.com wrote:
> 
> > From: J Freyensee <james_p_freyensee@linux.intel.com>
> > 
> > This allows drivers who call this function to be compiled modularly.
> > Otherwise, a driver who is interested in this type of functionality
> > has to implement their own get_task_comm() call, causing code
> > duplication in the Linux source tree.
> > 
> > Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
> 
> Third time:
> 
> Acked-by: David Rientjes <rientjes@google.com>

Okay, so you want me to add an 'acked-by' next time on this patch in my
patch-set series?

Re: [PATCH 1/4] export kernel call get_task_comm().

[David Rientjes]

On Thu, 5 May 2011, J Freyensee wrote:

> > Third time:
> > 
> > Acked-by: David Rientjes <rientjes@google.com>
> 
> Okay, so you want me to add an 'acked-by' next time on this patch in my
> patch-set series?
> 

That's the usual convention, yes.

[PATCH 1/4] export kernel call get_task_comm().

[james_p_freyensee]

From: J Freyensee <james_p_freyensee@linux.intel.com>

This allows drivers who call this function to be compiled modularly.
Otherwise, a driver who is interested in this type of functionality
has to implement their own get_task_comm() call, causing code
duplication in the Linux source tree.

Signed-off-by: J Freyensee <james_p_freyensee@linux.intel.com>
Acked-by: David Rientjes <rientjes@google.com>
---
 fs/exec.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 8328beb..e1ac338 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1004,6 +1004,7 @@ char *get_task_comm(char *buf, struct task_struct *tsk)
 	task_unlock(tsk);
 	return buf;
 }
+EXPORT_SYMBOL_GPL(get_task_comm);
 
 void set_task_comm(struct task_struct *tsk, char *buf)
 {
-- 
1.7.2.3