Google Groups no longer supports new Usenet posts or subscriptions. Historical content remains viewable.
Dismiss

Re: [tip:tracing/urgent] tracing: Add pipe_close interface

0 views
Skip to first unread message

KOSAKI Motohiro

unread,
Dec 9, 2009, 1:40:02 AM12/9/09
to
> Commit-ID: c521efd1700a8c0f7ce26f011f5eaecca17fabfa
> Gitweb: http://git.kernel.org/tip/c521efd1700a8c0f7ce26f011f5eaecca17fabfa
> Author: Steven Rostedt <sros...@redhat.com>
> AuthorDate: Mon, 7 Dec 2009 09:06:24 -0500
> Committer: Steven Rostedt <ros...@goodmis.org>
> CommitDate: Mon, 7 Dec 2009 12:01:35 -0500
>
> tracing: Add pipe_close interface
>
> An ftrace plugin can add a pipe_open interface when the user opens
> trace_pipe. But if the plugin allocates something within the pipe_open
> it can not free it because there exists no pipe_close. The hook to
> the trace file open has a corresponding close. The closing of the
> trace_pipe file should also have a corresponding close.
>
> Signed-off-by: Steven Rostedt <ros...@goodmis.org>
> ---
> kernel/trace/trace.c | 4 ++++
> kernel/trace/trace.h | 2 ++
> 2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 874f289..f804b40 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2898,6 +2898,10 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
> else
> cpumask_clear_cpu(iter->cpu_file, tracing_reader_cpumask);
>
> +
> + if (iter->trace->pipe_open)
> + iter->trace->pipe_close(iter);
> +

What's happen if pipe_close is NULL? Wny following straightforward check
is wrong?
I mean the above description explain pipe_close is only useful if plugin
allocate something at pipe_open. then allowing NULL seems natural.

if (iter->trace->pipe_close)
iter->trace->pipe_close(iter);

just nit.


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Steven Rostedt

unread,
Dec 9, 2009, 9:10:02 AM12/9/09
to
On Wed, 2009-12-09 at 15:35 +0900, KOSAKI Motohiro wrote:

> > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > index 874f289..f804b40 100644
> > --- a/kernel/trace/trace.c
> > +++ b/kernel/trace/trace.c
> > @@ -2898,6 +2898,10 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
> > else
> > cpumask_clear_cpu(iter->cpu_file, tracing_reader_cpumask);
> >
> > +
> > + if (iter->trace->pipe_open)
> > + iter->trace->pipe_close(iter);
> > +
>
> What's happen if pipe_close is NULL? Wny following straightforward check
> is wrong?
> I mean the above description explain pipe_close is only useful if plugin
> allocate something at pipe_open. then allowing NULL seems natural.
>
> if (iter->trace->pipe_close)
> iter->trace->pipe_close(iter);

Ug, good point (stupid cut & paste should be illegal).

Ingo, I'll fix this up too in the next patch set. Want me to rebase it
or just start with this fix?

-- Steve

Ingo Molnar

unread,
Dec 9, 2009, 9:20:01 AM12/9/09
to

* Steven Rostedt <ros...@goodmis.org> wrote:

> On Wed, 2009-12-09 at 15:35 +0900, KOSAKI Motohiro wrote:
>
> > > diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> > > index 874f289..f804b40 100644
> > > --- a/kernel/trace/trace.c
> > > +++ b/kernel/trace/trace.c
> > > @@ -2898,6 +2898,10 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
> > > else
> > > cpumask_clear_cpu(iter->cpu_file, tracing_reader_cpumask);
> > >
> > > +
> > > + if (iter->trace->pipe_open)
> > > + iter->trace->pipe_close(iter);
> > > +
> >
> > What's happen if pipe_close is NULL? Wny following straightforward check
> > is wrong?
> > I mean the above description explain pipe_close is only useful if plugin
> > allocate something at pipe_open. then allowing NULL seems natural.
> >
> > if (iter->trace->pipe_close)
> > iter->trace->pipe_close(iter);
>
> Ug, good point (stupid cut & paste should be illegal).
>
> Ingo, I'll fix this up too in the next patch set. Want me to rebase it
> or just start with this fix?

I'd suggest to start with a fix. Thanks,

Ingo

tip-bot for Steven Rostedt

unread,
Dec 10, 2009, 2:50:01 AM12/10/09
to
Commit-ID: 29bf4a5e3fed3dde3eb629a0cb1762c1e9217458
Gitweb: http://git.kernel.org/tip/29bf4a5e3fed3dde3eb629a0cb1762c1e9217458
Author: Steven Rostedt <sros...@redhat.com>
AuthorDate: Wed, 9 Dec 2009 12:37:43 -0500
Committer: Steven Rostedt <ros...@goodmis.org>
CommitDate: Wed, 9 Dec 2009 12:47:35 -0500

tracing: Only call pipe_close if pipe_close is defined

This fixes a cut and paste error that had pipe_close get called
if pipe_open was defined (not pipe_close).

Reported-by: Kosaki Motohiro <kosaki....@jp.fujitsu.com>
LKML-Reference: <20091209153204...@jp.fujitsu.com>


Signed-off-by: Steven Rostedt <ros...@goodmis.org>
---

kernel/trace/trace.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index f804b40..dc937e1 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2899,7 +2899,7 @@ static int tracing_release_pipe(struct inode *inode, struct file *file)
cpumask_clear_cpu(iter->cpu_file, tracing_reader_cpumask);


- if (iter->trace->pipe_open)
+ if (iter->trace->pipe_close)
iter->trace->pipe_close(iter);

mutex_unlock(&trace_types_lock);

0 new messages