2013-03-05 15:50:12

by Steven Rostedt

[permalink] [raw]
Subject: snapshot error on non allocated buffer?

Hi Hiraku,

I'm doing a lot of reconstruction of ftrace's buffering, and I'm also
modifying a lot of the snapshot feature to work with the new stuff
that's coming.

I'm looking at the -EINVAL when you write something other than '0' or
'1' into the snapshot file when the snapshot is not allocated. I'm
thinking that it should just return as if it succeeded. I don't
understand why it should return -EINVAL?

Now if you want to know if the snapshot is allocated or not, I have a
patch that shows how to use the snapshot feature when the snapshot is
empty, and also give the status of the snapshot itself:

[root] # cat /debug/tracing/snapshot
# tracer: nop
#
#
# * Snapshot is freed *
#
# Snapshot commands:
# echo 0 > snapshot : Clears and frees snapshot buffer
# echo 1 > snapshot : Allocates snapshot buffer, if not already
allocated.
# Takes a snapshot of the main buffer.
# echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
# (Doesn't have to be '2' works with any number
that
# is not a '0' or '1')

[root] # echo 1 > /debug/tracing/snapshot
[root] # echo 2 > /debug/tracing/snapshot
[root] # cat /debug/tracing/snapshot
# tracer: nop
#
#
# * Snapshot is allocated *
#
# Snapshot commands:
# echo 0 > snapshot : Clears and frees snapshot buffer
# echo 1 > snapshot : Allocates snapshot buffer, if not already
allocated.
# Takes a snapshot of the main buffer.
# echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
# (Doesn't have to be '2' works with any number
that
# is not a '0' or '1')


As this is a new feature for 3.9, and we are still in -rc1, I think this
might be a good thing to add now. As well as not returning -EINVAL on
writing to the file when the snapshot buffer isn't allocated.

What do you think?

-- Steve


2013-03-06 11:57:52

by Hiraku Toyooka

[permalink] [raw]
Subject: Re: snapshot error on non allocated buffer?

Hi Steven,

(03/06/2013 12:50 AM), Steven Rostedt wrote:
> Hi Hiraku,
>
> I'm doing a lot of reconstruction of ftrace's buffering, and I'm also
> modifying a lot of the snapshot feature to work with the new stuff
> that's coming.
>

Many thanks. I'm trying your multi-buffer patches.

> I'm looking at the -EINVAL when you write something other than '0' or
> '1' into the snapshot file when the snapshot is not allocated. I'm
> thinking that it should just return as if it succeeded. I don't
> understand why it should return -EINVAL?
>

I thought that it might be a little strange if the clear operation
succeeded in spite of the non-allocated buffer.
(Actually, I simply implemented as you said, though.)

But I don't have trouble even if it succeeds, so I'll modify the I/F
to make it return successfully.

> Now if you want to know if the snapshot is allocated or not, I have a
> patch that shows how to use the snapshot feature when the snapshot is
> empty, and also give the status of the snapshot itself:
>
> [root] # cat /debug/tracing/snapshot
> # tracer: nop
> #
> #
> # * Snapshot is freed *
> #
> # Snapshot commands:
> # echo 0 > snapshot : Clears and frees snapshot buffer
> # echo 1 > snapshot : Allocates snapshot buffer, if not already
> allocated.
> # Takes a snapshot of the main buffer.
> # echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
> # (Doesn't have to be '2' works with any number
> that
> # is not a '0' or '1')
>
> [root] # echo 1 > /debug/tracing/snapshot
> [root] # echo 2 > /debug/tracing/snapshot
> [root] # cat /debug/tracing/snapshot
> # tracer: nop
> #
> #
> # * Snapshot is allocated *
> #
> # Snapshot commands:
> # echo 0 > snapshot : Clears and frees snapshot buffer
> # echo 1 > snapshot : Allocates snapshot buffer, if not already
> allocated.
> # Takes a snapshot of the main buffer.
> # echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
> # (Doesn't have to be '2' works with any number
> that
> # is not a '0' or '1')
>

This seems good for me and also users.

>
> As this is a new feature for 3.9, and we are still in -rc1, I think this
> might be a good thing to add now. As well as not returning -EINVAL on
> writing to the file when the snapshot buffer isn't allocated.
>
> What do you think?
>

I think it's OK.
I'll send a patch to make the file not return -EINVAL. Does it need to
be based on 3.9-rc1 or tip tree?

Thanks,
Hiraku Toyooka

2013-03-06 12:43:11

by Steven Rostedt

[permalink] [raw]
Subject: Re: snapshot error on non allocated buffer?

On Wed, 2013-03-06 at 20:54 +0900, Hiraku Toyooka wrote:
> Hi Steven,
>
> (03/06/2013 12:50 AM), Steven Rostedt wrote:
> > Hi Hiraku,
> >
> > I'm doing a lot of reconstruction of ftrace's buffering, and I'm also
> > modifying a lot of the snapshot feature to work with the new stuff
> > that's coming.
> >
>
> Many thanks. I'm trying your multi-buffer patches.

And I have many more to come :-)

>
> > I'm looking at the -EINVAL when you write something other than '0' or
> > '1' into the snapshot file when the snapshot is not allocated. I'm
> > thinking that it should just return as if it succeeded. I don't
> > understand why it should return -EINVAL?
> >
>
> I thought that it might be a little strange if the clear operation
> succeeded in spite of the non-allocated buffer.
> (Actually, I simply implemented as you said, though.)

Yeah, I may have been the one to bring it up, but I was wrong. After
playing with it, it doesn't make sense.

>
> But I don't have trouble even if it succeeds, so I'll modify the I/F
> to make it return successfully.
>
> > Now if you want to know if the snapshot is allocated or not, I have a
> > patch that shows how to use the snapshot feature when the snapshot is
> > empty, and also give the status of the snapshot itself:
> >
> > [root] # cat /debug/tracing/snapshot
> > # tracer: nop
> > #
> > #
> > # * Snapshot is freed *
> > #
> > # Snapshot commands:
> > # echo 0 > snapshot : Clears and frees snapshot buffer
> > # echo 1 > snapshot : Allocates snapshot buffer, if not already
> > allocated.
> > # Takes a snapshot of the main buffer.
> > # echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
> > # (Doesn't have to be '2' works with any number
> > that
> > # is not a '0' or '1')
> >
> > [root] # echo 1 > /debug/tracing/snapshot
> > [root] # echo 2 > /debug/tracing/snapshot
> > [root] # cat /debug/tracing/snapshot
> > # tracer: nop
> > #
> > #
> > # * Snapshot is allocated *
> > #
> > # Snapshot commands:
> > # echo 0 > snapshot : Clears and frees snapshot buffer
> > # echo 1 > snapshot : Allocates snapshot buffer, if not already
> > allocated.
> > # Takes a snapshot of the main buffer.
> > # echo 2 > snapshot : Clears snapshot buffer (but does not allocate)
> > # (Doesn't have to be '2' works with any number
> > that
> > # is not a '0' or '1')
> >
>
> This seems good for me and also users.
>
> >
> > As this is a new feature for 3.9, and we are still in -rc1, I think this
> > might be a good thing to add now. As well as not returning -EINVAL on
> > writing to the file when the snapshot buffer isn't allocated.
> >
> > What do you think?
> >
>
> I think it's OK.
> I'll send a patch to make the file not return -EINVAL. Does it need to
> be based on 3.9-rc1 or tip tree?

I already have a patch. I'll send it out later today, and perhaps you
can give me your "Acked-by".

Thanks,

-- Steve