From: J Freyensee <[email protected]>
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 <[email protected]>
---
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
On Tue, 19 Apr 2011, [email protected] wrote:
> From: J Freyensee <[email protected]>
>
> 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 <[email protected]>
Acked-by: David Rientjes <[email protected]>
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!
On Tue, 2011-04-19 at 16:22 -0700, David Rientjes wrote:
> On Tue, 19 Apr 2011, [email protected] wrote:
>
> > From: J Freyensee <[email protected]>
> >
> > 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 <[email protected]>
>
> Acked-by: David Rientjes <[email protected]>
>
> 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?
On Tue, 19 Apr 2011, J Freyensee wrote:
> > > From: J Freyensee <[email protected]>
> > >
> > > 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 <[email protected]>
> >
> > Acked-by: David Rientjes <[email protected]>
> >
> > 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.
On Tue, Apr 19, 2011 at 06:22:14PM -0700, David Rientjes wrote:
> On Tue, 19 Apr 2011, J Freyensee wrote:
>
> > > > From: J Freyensee <[email protected]>
> > > >
> > > > 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 <[email protected]>
> > >
> > > Acked-by: David Rientjes <[email protected]>
> > >
> > > 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
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 <[email protected]>
> > > > >
> > > > > 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 <[email protected]>
> > > >
> > > > Acked-by: David Rientjes <[email protected]>
> > > >
> > > > 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
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 <[email protected]>
> > > > > >
> > > > > > 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 <[email protected]>
> > > > >
> > > > > Acked-by: David Rientjes <[email protected]>
> > > > >
> > > > > 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
On Wed, 20 Apr 2011 12:14:21 -0700
Greg KH <[email protected]> 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
On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> On Wed, 20 Apr 2011 12:14:21 -0700
> Greg KH <[email protected]> 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
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.
On Wed, 20 Apr 2011 16:16:08 -0700
J Freyensee <[email protected]> wrote:
> On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> > On Wed, 20 Apr 2011 12:14:21 -0700
> > Greg KH <[email protected]> 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)
On Wed, 2011-04-20 at 16:46 -0700, Andrew Morton wrote:
> On Wed, 20 Apr 2011 16:16:08 -0700
> J Freyensee <[email protected]> wrote:
>
> > On Wed, 2011-04-20 at 16:11 -0700, Andrew Morton wrote:
> > > On Wed, 20 Apr 2011 12:14:21 -0700
> > > Greg KH <[email protected]> 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)