Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1530512 > unrolled thread

Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-25 21:00 +0100
    Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Mike Marshall <hubcap@omnibond.com> - 2016-11-25 21:40 +0100
    Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Theodore Ts'o <tytso@mit.edu> - 2016-11-25 22:50 +0100
      Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-26 00:40 +0100
      Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Jan Kara <jack@suse.cz> - 2016-11-28 09:40 +0100
    Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Dave Chinner <david@fromorbit.com> - 2016-11-27 23:50 +0100
      Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Linus Torvalds <torvalds@linux-foundation.org> - 2016-11-28 02:00 +0100
        Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Al Viro <viro@ZenIV.linux.org.uk> - 2016-11-28 03:10 +0100
        Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing Dave Chinner <david@fromorbit.com> - 2016-11-28 10:10 +0100

#1530512 — Re: [PATCH 3/6] dax: add tracepoint infrastructure, PMD tracing

On Thu, Nov 24, 2016 at 11:37 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> My impression is that nobody (at least kernel-side) wants them to be
> a stable ABI, so long as nobody in userland screams about their code
> being broken, everything is fine.  As usual, if nobody notices an ABI
> change, it hasn't happened.  The question is what happens when somebody
> does.

Right. There is basically _no_ "stable API" for the kernel anywhere,
it's just an issue of "you can't break workflow for normal people".

And if somebody writes his own trace scripts, and some random trace
point goes away (or changes semantics), that's easy: he can just fix
his script. Tracepoints aren't ever going to be stable in that sense.

But when then somebody writes a trace script that is so useful that
distros pick it up, and people start using it and depending on it,
then _that_ trace point may well have become effectively locked in
stone.

That's happened once already with the whole powertop thing. It didn't
get all that widely spread, and the fix was largely to improve on
powertop to the point where it wasn't a problem any more, but we've
clearly had one case of this happening.

But I suspect that something like powertop is fairly unusual. There is
certainly room for similar things in the VFS layer (think "better
vmstat that uses tracepoints"), but I suspect the bulk of tracepoints
are going to be for random debugging (so that developers can say
"please run this script") rather than for an actual user application
kind of situation.

So I don't think we should be _too_ afraid of tracepoints either. When
being too anti-tracepoint is a bigger practical problem than the
possible problems down the line, the balance is wrong.

As long as tracepoints make sense from a higher standpoint (ie not
just random implementation detail of the day), and they aren't
everywhere, they are unlikely to cause much problem.

We do have filesystem code that is just disgusting. As an example:
fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
single function. If you want that, use the function tracer. That seems
to be just debugging code that has been left around for others to
stumble over. I do *not* believe that we should encourage that kind of
"machine gun spray" use of tracepoints.

But tracing actual high-level things like IO and faults? I think that
makes perfect sense, as long as the data that is collected is also the
actual event data, and not so much a random implementation issue of
the day.

             Linus

[toc] | [next] | [standalone]

#1530521

> We do have filesystem code that is just disgusting. As an example:
> fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
> single function.

Hmmm... we have "gossip" statements in Orangefs which can be triggered with
a debugfs knob... lots of functions have a gossip statement at the
top... is that
disgusting?

-Mike

On Fri, Nov 25, 2016 at 2:51 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Nov 24, 2016 at 11:37 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>>
>> My impression is that nobody (at least kernel-side) wants them to be
>> a stable ABI, so long as nobody in userland screams about their code
>> being broken, everything is fine.  As usual, if nobody notices an ABI
>> change, it hasn't happened.  The question is what happens when somebody
>> does.
>
> Right. There is basically _no_ "stable API" for the kernel anywhere,
> it's just an issue of "you can't break workflow for normal people".
>
> And if somebody writes his own trace scripts, and some random trace
> point goes away (or changes semantics), that's easy: he can just fix
> his script. Tracepoints aren't ever going to be stable in that sense.
>
> But when then somebody writes a trace script that is so useful that
> distros pick it up, and people start using it and depending on it,
> then _that_ trace point may well have become effectively locked in
> stone.
>
> That's happened once already with the whole powertop thing. It didn't
> get all that widely spread, and the fix was largely to improve on
> powertop to the point where it wasn't a problem any more, but we've
> clearly had one case of this happening.
>
> But I suspect that something like powertop is fairly unusual. There is
> certainly room for similar things in the VFS layer (think "better
> vmstat that uses tracepoints"), but I suspect the bulk of tracepoints
> are going to be for random debugging (so that developers can say
> "please run this script") rather than for an actual user application
> kind of situation.
>
> So I don't think we should be _too_ afraid of tracepoints either. When
> being too anti-tracepoint is a bigger practical problem than the
> possible problems down the line, the balance is wrong.
>
> As long as tracepoints make sense from a higher standpoint (ie not
> just random implementation detail of the day), and they aren't
> everywhere, they are unlikely to cause much problem.
>
> We do have filesystem code that is just disgusting. As an example:
> fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
> single function. If you want that, use the function tracer. That seems
> to be just debugging code that has been left around for others to
> stumble over. I do *not* believe that we should encourage that kind of
> "machine gun spray" use of tracepoints.
>
> But tracing actual high-level things like IO and faults? I think that
> makes perfect sense, as long as the data that is collected is also the
> actual event data, and not so much a random implementation issue of
> the day.
>
>              Linus
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1530547

On Fri, Nov 25, 2016 at 11:51:26AM -0800, Linus Torvalds wrote:
> We do have filesystem code that is just disgusting. As an example:
> fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
> single function. If you want that, use the function tracer. That seems
> to be just debugging code that has been left around for others to
> stumble over. I do *not* believe that we should encourage that kind of
> "machine gun spray" use of tracepoints.

There is a reason why people want to be able to do that, and that's
because kprobes doesn't give you access to the arguments and return
codes to the functions.  Maybe there could be a way to do this more
easily using DWARF information and EBPF magic, perhaps?  It won't help
for inlined functions, of course, but most of the functions where
people want to do this aren't generally functions which are going to
be inlined, but rather things like write_begin, writepages, which are
called via a struct ops table and so will never be inlined to begin
with.

And it *is* handy to be able to do this when you don't know ahead of
time that you might need to debug a production system that is
malfunctioning for some reason.  This is the "S" in RAS (Reliability,
Availability, Serviceability).  This is why it's nice if there were a
way to be clear that it is intended for debugging purposes only ---
and maybe kprobes with EBPF and DWARF would be the answer.

After all, we need *some* way of saying this can never be considered
stable --- what would we do if some userspace program like powertop
started depending on a function name via ktrace and that function
disappeared --- would the userspace application really be intended to
demand that we revert the recatoring, because eliminating a function
name that they were depending on via ktrace point broke them?

						- Ted

[toc] | [prev] | [next] | [standalone]

#1530570

On Fri, Nov 25, 2016 at 1:48 PM, Theodore Ts'o <tytso@mit.edu> wrote:
>
> There is a reason why people want to be able to do that, and that's
> because kprobes doesn't give you access to the arguments and return
> codes to the functions.

Honestly, that's simply not a good reason.

What if everybody did this? Do we pollute the whole kernel with this crap? No.

And if not, then what's so special about something like afs that it
would make sense there?

The thing is, with function tracing, you *can* get the return value
and arguments. Sure, you'll probably need to write eBPF and just
attach it to that fentry call point, and yes, if something is inlined
you're just screwed, but Christ, if you do debugging that way you
shouldn't be writing kernel code in the first place.

If you cannot do filesystem debugging without tracing every single
function entry, you are doing something seriously wrong. Add a couple
of relevant and valid trace points to get the initial arguments etc
(and perhaps to turn on the function tracing going down the stack).

> After all, we need *some* way of saying this can never be considered
> stable.

Oh, if you pollute the kernel with random idiotic trace points, not
only are they not going to be considered stable, after a while people
should stop pulling from you.

I do think we should probably add a few generic VFS level breakpoints
to make it easier for people to catch the arguments they get from the
VFS layer (not every system call - if you're a filesystem person, you
_shouldn't_ care about all the stuff that the VFS layer caches for you
so that you never even have to see it). I do think that Al's "no trace
points what-so-ever" is too strict.

But I think a lot of people add complete crap with the "maybe it's
needed some day" kind of mentality.

The tracepoints should have a good _specific_ reason, and they should
make sense. Not be randomly sprinkled "just because".

             Linus

[toc] | [prev] | [next] | [standalone]

#1531115

On Fri 25-11-16 16:48:40, Ted Tso wrote:
> On Fri, Nov 25, 2016 at 11:51:26AM -0800, Linus Torvalds wrote:
> > We do have filesystem code that is just disgusting. As an example:
> > fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
> > single function. If you want that, use the function tracer. That seems
> > to be just debugging code that has been left around for others to
> > stumble over. I do *not* believe that we should encourage that kind of
> > "machine gun spray" use of tracepoints.
> 
> There is a reason why people want to be able to do that, and that's
> because kprobes doesn't give you access to the arguments and return
> codes to the functions.  Maybe there could be a way to do this more
> easily using DWARF information and EBPF magic, perhaps?  It won't help
> for inlined functions, of course, but most of the functions where
> people want to do this aren't generally functions which are going to
> be inlined, but rather things like write_begin, writepages, which are
> called via a struct ops table and so will never be inlined to begin
> with.

Actually, you can print register & stack contents from a kprobe and you can
get a function return value from a kretprobe (see
Documentation/trace/kprobetrace.txt). Since calling convention is fixed
(arg 1 in RDI, arg 2 in RSI...) you can relatively easily dump function
arguments on entry and dump return value on return for arbitrary function
of your choice. I was already debugging issues like that several times (in
VFS actually because of missing trace points ;)). You can even create a
kprobe to dump register contents in the middle of the function (although
there it takes more effort reading the dissasembly to see what you are
interested in).

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[toc] | [prev] | [next] | [standalone]

#1530953

On Fri, Nov 25, 2016 at 11:51:26AM -0800, Linus Torvalds wrote:
> On Thu, Nov 24, 2016 at 11:37 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > My impression is that nobody (at least kernel-side) wants them to be
> > a stable ABI, so long as nobody in userland screams about their code
> > being broken, everything is fine.  As usual, if nobody notices an ABI
> > change, it hasn't happened.  The question is what happens when somebody
> > does.
> 
> Right. There is basically _no_ "stable API" for the kernel anywhere,
> it's just an issue of "you can't break workflow for normal people".
> 
> And if somebody writes his own trace scripts, and some random trace
> point goes away (or changes semantics), that's easy: he can just fix
> his script. Tracepoints aren't ever going to be stable in that sense.
> 
> But when then somebody writes a trace script that is so useful that
> distros pick it up, and people start using it and depending on it,
> then _that_ trace point may well have become effectively locked in
> stone.

And that's exactly why we need a method of marking tracepoints as
stable. How else are we going to know whether a specific tracepoint
is stable if the kernel code doesn't document that it's stable? And
how are we going to know why it's considered stable if there isn't a
commit message that explains why it was made stable?

> We do have filesystem code that is just disgusting. As an example:
> fs/afs/ tends to have these crazy "_enter()/_exit()" macros in every
> single function. If you want that, use the function tracer. That seems
> to be just debugging code that has been left around for others to
> stumble over. I do *not* believe that we should encourage that kind of
> "machine gun spray" use of tracepoints.

Inappropriate use of tracepoints is a different problem. The issue
here is getting rid of the uncertainty caused by the handwavy
"tracepoints a mutable until someone, somewhere decides to use it in
userspace" policy.

> But tracing actual high-level things like IO and faults? I think that
> makes perfect sense, as long as the data that is collected is also the
> actual event data, and not so much a random implementation issue of
> the day.

IME, a tracepoint that doesn't expose detailed context specific
information isn't really useful for complex problem diagnosis...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [next] | [standalone]

#1530976

On Sun, Nov 27, 2016 at 2:42 PM, Dave Chinner <david@fromorbit.com> wrote:
>
> And that's exactly why we need a method of marking tracepoints as
> stable. How else are we going to know whether a specific tracepoint
> is stable if the kernel code doesn't document that it's stable?

You are living in some unrealistic dream-world where you think you can
get the right tracepoint on the first try.

So there is no way in hell I would ever mark any tracepoint "stable"
until it has had a fair amount of use, and there are useful tools that
actually make use of it, and it has shown itself to be the right
trace-point.

And once that actually happens, what's the advantage of marking it
stable? None. It's a catch-22. Before it has uses and has been tested
and found to be good, it's not stable. And after, it's pointless.

So at no point does such a "stable" tracepoint marking make sense. At
most, you end up adding a comment saying "this tracepoint is used by
tools such-and-such".

                   Linus

[toc] | [prev] | [next] | [standalone]

#1530985

On Sun, Nov 27, 2016 at 04:58:43PM -0800, Linus Torvalds wrote:
> You are living in some unrealistic dream-world where you think you can
> get the right tracepoint on the first try.
> 
> So there is no way in hell I would ever mark any tracepoint "stable"
> until it has had a fair amount of use, and there are useful tools that
> actually make use of it, and it has shown itself to be the right
> trace-point.
> 
> And once that actually happens, what's the advantage of marking it
> stable? None. It's a catch-22. Before it has uses and has been tested
> and found to be good, it's not stable. And after, it's pointless.
> 
> So at no point does such a "stable" tracepoint marking make sense. At
> most, you end up adding a comment saying "this tracepoint is used by
> tools such-and-such".

I can't speak for Dave, but I suspect that it's more about "this, this and
that tracepoints are purely internal and we can and will change them whenever
we bloody feel like that; stick your fingers in those and they _will_ get
crushed".

Incidentally, take a look at
        trace_ocfs2_file_aio_read(inode, filp, filp->f_path.dentry,
                        (unsigned long long)OCFS2_I(inode)->ip_blkno,
                        filp->f_path.dentry->d_name.len,
                        filp->f_path.dentry->d_name.name,
                        to->nr_segs);   /* GRRRRR */
Note that there is nothing whatsoever protecting the use of ->d_name in
there (not that poking in iov_iter guts was a good idea).  Besides, suppose
something *did* grab a hold of that one a while ago.  What would we have
to do to avoid stepping on its toes every time when somebody call ocfs2
->splice_read(), which has recently started to go through ->read_iter()
calls?  Prepend something like if (!(to->type & ITER_PIPE)) to it?

I'm very tempted to just go and remove it, along with its analogues.
If nothing else, the use of ->d_name *is* racy, and while it might be
tolerable for occasional debugging, for anything in heavier use it's
asking for trouble...

[toc] | [prev] | [next] | [standalone]

#1531139

On Sun, Nov 27, 2016 at 04:58:43PM -0800, Linus Torvalds wrote:
> On Sun, Nov 27, 2016 at 2:42 PM, Dave Chinner <david@fromorbit.com> wrote:
> >
> > And that's exactly why we need a method of marking tracepoints as
> > stable. How else are we going to know whether a specific tracepoint
> > is stable if the kernel code doesn't document that it's stable?
> 
> You are living in some unrealistic dream-world where you think you can
> get the right tracepoint on the first try.

No, I'm  under no illusions that we'd get stable tracepoints right
the first go. I don't care about how we stabilise stable
tracepoints, because nothing I maintain will use stable tracepoints.
However, I will point out that we have /already solved these ABI
development problems/.

$ ls Documentation/ABI/
obsolete  README  removed  stable  testing
$

Expands this to include stable tracepoints, not just sysfs files.
New stable stuff gets classified as "testing" meaning it is supposed
to be stable but may change before being declared officially stable.
"stable" is obvious, are "obsolete" and "removed".


> So there is no way in hell I would ever mark any tracepoint "stable"
> until it has had a fair amount of use, and there are useful tools that
> actually make use of it, and it has shown itself to be the right
> trace-point.
> 
> And once that actually happens, what's the advantage of marking it
> stable? None. It's a catch-22. Before it has uses and has been tested
> and found to be good, it's not stable. And after, it's pointless.

And that "catch-22" is *precisely the problem we need to solve*.

Pointing out that there's a catch-22 doesn't help anyone - all the
developers that are telling you that they really need a way to mark
stable tracepoints already understand this catch-22 and they want a
way to avoid it.  Being able to either say "this is stable and we'll
support it forever" or "this will never be stable so use at your own
risk" is a simple way of avoiding the catch-22. If an unstable
tracepoint is useful to applications *and it can be implemented in a
maintainable, stable form* then it can go through the process of
being made stable and documented in Documentation/ABI/stable.

Problem is solved, catch-22 is gone.

All we want is some method of making a clear, unambiguous statement
about the nature of a specific tracepoint and a process for
transitioning a tracepoint to a stable, maintainable form. We do it
for other ABI interfaces, so why can't we do this for tracepoints?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web