All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ted Ts'o" <tytso@mit.edu>
To: John Stultz <john.stultz@linaro.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Rientjes <rientjes@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Dave Hansen <dave@linux.vnet.ibm.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH 3/3] comm: ext4: Protect task->comm access by using get_task_comm()
Date: Sat, 7 May 2011 12:30:50 -0400	[thread overview]
Message-ID: <20110507163050.GA6046@thunk.org> (raw)
In-Reply-To: <1304553310.2943.18.camel@work-vm>

On Wed, May 04, 2011 at 04:55:10PM -0700, John Stultz wrote:
> > I'm suspecting that approximately 100% of the get_task_comm() callsites
> > are using it for a printk, so how about we add a %p thingy for it then
> > zap lots of code?
> 
> DaveH suggested the same, actually. And that would work with the
> seqlocking pretty easily to avoid DavidR's issue.

+1 for a %p thingy for printk's; although the other potential use case
that we should think about is for tracepoints.  Getting something that
works for ftrace as well as perf would be a really good thing.

I suspect what we would want to do though (since people have been
trying very hard to keep the trace records as small as possible, so we
can include as much as possible) is to only record the pid, and have a
tracepoint which reports when process's comm value has been set to a
new value.  So any objections to adding a tracepoint in
set_task_comm()?

And would you like me to send the patch, or do you want to do it since
you're putting a patch series together anyway?

    	      	      	      	  	    - Ted


WARNING: multiple messages have this Message-ID (diff)
From: Ted Ts'o <tytso@mit.edu>
To: John Stultz <john.stultz@linaro.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	David Rientjes <rientjes@google.com>,
	LKML <linux-kernel@vger.kernel.org>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	Dave Hansen <dave@linux.vnet.ibm.com>,
	linux-mm@kvack.org
Subject: Re: [PATCH 3/3] comm: ext4: Protect task->comm access by using get_task_comm()
Date: Sat, 7 May 2011 12:30:50 -0400	[thread overview]
Message-ID: <20110507163050.GA6046@thunk.org> (raw)
In-Reply-To: <1304553310.2943.18.camel@work-vm>

On Wed, May 04, 2011 at 04:55:10PM -0700, John Stultz wrote:
> > I'm suspecting that approximately 100% of the get_task_comm() callsites
> > are using it for a printk, so how about we add a %p thingy for it then
> > zap lots of code?
> 
> DaveH suggested the same, actually. And that would work with the
> seqlocking pretty easily to avoid DavidR's issue.

+1 for a %p thingy for printk's; although the other potential use case
that we should think about is for tracepoints.  Getting something that
works for ftrace as well as perf would be a really good thing.

I suspect what we would want to do though (since people have been
trying very hard to keep the trace records as small as possible, so we
can include as much as possible) is to only record the pid, and have a
tracepoint which reports when process's comm value has been set to a
new value.  So any objections to adding a tracepoint in
set_task_comm()?

And would you like me to send the patch, or do you want to do it since
you're putting a patch series together anyway?

    	      	      	      	  	    - Ted

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-05-07 16:30 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-04-28  4:03 [RFC][PATCH 0/3] Improve task->comm locking situation John Stultz
2011-04-28  4:03 ` John Stultz
2011-04-28  4:03 ` [PATCH 1/3] comm: Introduce comm_lock seqlock to protect task->comm access John Stultz
2011-04-28  4:03   ` John Stultz
2011-04-28  4:03 ` [PATCH 2/3] comm: timerstats: Protect task->comm access by using get_task_comm() John Stultz
2011-04-28  4:03   ` John Stultz
2011-04-28  4:03 ` [PATCH 3/3] comm: ext4: " John Stultz
2011-04-28  4:03   ` John Stultz
2011-04-28 21:35   ` David Rientjes
2011-04-28 21:35     ` David Rientjes
2011-05-04 23:36     ` Andrew Morton
2011-05-04 23:36       ` Andrew Morton
2011-05-04 23:42       ` Andrew Morton
2011-05-04 23:42         ` Andrew Morton
2011-05-04 23:55       ` John Stultz
2011-05-04 23:55         ` John Stultz
2011-05-07 16:30         ` Ted Ts'o [this message]
2011-05-07 16:30           ` Ted Ts'o

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20110507163050.GA6046@thunk.org \
    --to=tytso@mit.edu \
    --cc=akpm@linux-foundation.org \
    --cc=dave@linux.vnet.ibm.com \
    --cc=john.stultz@linaro.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
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.