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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dtrace-tracemem.patch
Type: application/octet-stream
Size: 94233 bytes
Desc: not available
URL:
<http://mail.opensolaris.org/pipermail/dtrace-discuss/attachments/20110711/470955e3/attachment-0001.obj>
G''Day, On Mon, Jul 11, 2011 at 11:33 PM, Bryan Cantrill <bryan at joyent.com> wrote:> 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 -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://mail.opensolaris.org/pipermail/dtrace-discuss/attachments/20110712/e39a075b/attachment.html>
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
On Mon, Jul 11, 2011 at 11:33 PM, Bryan Cantrill <bryan at joyent.com>
wrote:> 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
> dtrace-discuss at opensolaris.org
>
--
Adam Leventhal, Delphix
http://dtrace.org/blogs/ahl
275 Middlefield Road, Suite 50
Menlo Park, CA 94025
http://www.delphix.com
On Tue, Jul 12, 2011 at 4:54 PM, Adam Leventhal <ahl at delphix.com> wrote:> 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?> 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...> ?{ "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().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
Hey Bryan,> 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