Discussion:
dynamic size argument to tracemem()
Bryan Cantrill
2011-07-12 06:33:15 UTC
Permalink
All,

With my apologies for having been somewhat on a tear through old
DTrace RFEs, another that has been annoying as of late is that
tracemem() does not take a dynamic size. Of course, EPIDs must
correspond to a static size, so the fix is not to make the size
entirely dynamic, but rather to add an optional third argument to
tracemem() that is a D expression that captures the dynamic size --
provided that it is no greater than the static size (still) provided
as the second argument. A patch that implements this is attached for
your review and consideration; as with our other work, this will be
showing up at http://github.com/joyent/illumos-joyent in the near
future (our ticket for this is OS-481, "DTrace tracemem() action
should take a dynamic size argument"). Note the fix for another
long-standing falls out of this: if you tracemem() something that is
a string, it will show up as a proper hex dump instead of being
interpreted as a string. Please let me know if you have any questions
or comments!

Thanks,
Bryan
Brendan Gregg
2011-07-12 07:55:19 UTC
Permalink
G'Day,
Post by Bryan Cantrill
All,
With my apologies for having been somewhat on a tear through old
DTrace RFEs, another that has been annoying as of late is that
tracemem() does not take a dynamic size. Of course, EPIDs must
correspond to a static size, so the fix is not to make the size
entirely dynamic, but rather to add an optional third argument to
tracemem() that is a D expression that captures the dynamic size --
provided that it is no greater than the static size (still) provided
as the second argument. A patch that implements this is attached for
your review and consideration; as with our other work, this will be
showing up at http://github.com/joyent/illumos-joyent in the near
future (our ticket for this is OS-481, "DTrace tracemem() action
should take a dynamic size argument"). Note the fix for another
long-standing falls out of this: if you tracemem() something that is
a string, it will show up as a proper hex dump instead of being
interpreted as a string. Please let me know if you have any questions
or comments!
Many thanks!

If anyone is wondering about an example use case, I ran into this only a
couple of weeks ago when blogging about copyin(), writing:

"1st caveat: The "100" in the above one-liner is the length for tracemem();
unfortunately, it must be a scalar constant and can't be a variable,
otherwise I'd use "arg1" to print the entire returned length."

(The blog post was:
http://dtrace.org/blogs/brendan/2011/06/30/tweaking-memory-on-the-fly/)

This fix will let me pick a max-expected returned length for arg1, and then
use the extra arg to truncate it. I'll need to update the blog post. :-)

Brendan
--
Brendan Gregg, Joyent http://dtrace.org/blogs/brendan
Adam Leventhal
2011-07-12 23:54:08 UTC
Permalink
Hey Bryan,

Did you consider having the behavior be that if the third argument was
greater than the second (unsigned) that it would generate an error?

Regarding tst.dynsize.d, could the output fail to match if the tick
probe switched CPUs? Is that sufficiently unlikely, or would it be
worth using a profile probe with a cpu predicate (caching the first
value we see)?

{ "tracemem", DT_IDENT_ACTFUNC, 0, DT_ACT_TRACEMEM,
DT_ATTR_STABCMN, DT_VERS_1_0,
- &dt_idops_func, "void(@, size_t)" },
+ &dt_idops_func, "void(@, size_t, ...)" },

I think that should be: "void(@, size_t, [size_t])" like copyinstr() or index().

Looks great.

Adam
Post by Bryan Cantrill
All,
With my apologies for having been somewhat on a tear through old
DTrace RFEs, another that has been annoying as of late is that
tracemem() does not take a dynamic size.  Of course, EPIDs must
correspond to a static size, so the fix is not to make the size
entirely dynamic, but rather to add an optional third argument to
tracemem() that is a D expression that captures the dynamic size --
provided that it is no greater than the static size (still) provided
as the second argument.  A patch that implements this is attached for
your review and consideration; as with our other work, this will be
showing up at http://github.com/joyent/illumos-joyent in the near
future (our ticket for this is OS-481, "DTrace tracemem() action
should take a dynamic size argument").  Note the fix for another
long-standing falls out of this:  if you tracemem() something that is
a string, it will show up as a proper hex dump instead of being
interpreted as a string.  Please let me know if you have any questions
or comments!
       Thanks,
       Bryan
_______________________________________________
dtrace-discuss mailing list
--
Adam Leventhal, Delphix
http://dtrace.org/blogs/ahl

275 Middlefield Road, Suite 50
Menlo Park, CA 94025
http://www.delphix.com
Bryan Cantrill
2011-07-13 00:33:00 UTC
Permalink
Post by Adam Leventhal
Hey Bryan,
Did you consider having the behavior be that if the third argument was
greater than the second (unsigned) that it would generate an error?
Yeah, I did consider that and we had a conversation about that here.
The consensus was that implicitly clamping it was the cleaner behavior
-- that the closest analogue here was the clamping behavior of strsize
(where you don't see an error, just a silent clamping) and that you
don't want to see the console flooded with error messages in the case
where it's consistently larger than the static size. What are your
thoughts?
Post by Adam Leventhal
Regarding tst.dynsize.d, could the output fail to match if the tick
probe switched CPUs? Is that sufficiently unlikely, or would it be
worth using a profile probe with a cpu predicate (caching the first
value we see)?
Yeah, that's true -- but unlikely, as you say. FWIW, that's true in
many places in the test suite. In fact there was a port of DTrace out
there that changes the CPU on which the tick probe fires on every
firing, and the test suite fell over in a zillion places. So if this
is a problem that we want to tackle, I think we might want to do it
somewhat holistically...
Post by Adam Leventhal
 { "tracemem", DT_IDENT_ACTFUNC, 0, DT_ACT_TRACEMEM,
       DT_ATTR_STABCMN, DT_VERS_1_0,
I could have done in that way, but I'm actually doing the check
manually because I want to propagate my own error tag and test on it.

Thanks for taking a look at this -- and I only have two more wads to
come! (They're both tiny.)

- Bryan
Adam Leventhal
2011-07-13 00:37:18 UTC
Permalink
Hey Bryan,
Post by Bryan Cantrill
Yeah, I did consider that and we had a conversation about that here.
The consensus was that implicitly clamping it was the cleaner behavior
-- that the closest analogue here was the clamping behavior of strsize
(where you don't see an error, just a silent clamping) and that you
don't want to see the console flooded with error messages in the case
where it's consistently larger than the static size.  What are your
thoughts?
I guess I can see it both ways. On one hand, I might just care about
the first N bytes of a structure, and on the other I might want to
know that I've failed to record the data I'm trying to trace. Could we
treat it like a drop so you're not flooded with errors? I think I'd
prefer that it generate some kind of warning output -- it would be
very easy for users to do tracemem(`foo, 128, MAX(size, 128)); if they
wanted the error ignored.

Adam
--
Adam Leventhal, Delphix
http://dtrace.org/blogs/ahl

275 Middlefield Road, Suite 50
Menlo Park, CA 94025
http://www.delphix.com
Loading...