On Tue, Jul 12, 2011 at 5:25 PM, Adam Leventhal <a...@delphix.com> wrote: >>> I actually meant a negative test case. >> >> As I imagine you saw, there's already one there that uses speculative >> tracing and verifies that enablings are not reaped: >> >> usr/src/cmd/dtrace/test/tst/common/usdt/tst.noreap.ksh >> >> Having a test that uses ring buffering is certainly possible, it's >> just a more complicated test to write. (I'm currently cheating a bit >> by using one enabling to kick off another.) But I'm happy to add it >> if you'd like to see it... > > Yes. I saw that test case. It was just a suggestion to exercise the > other path in your code, but it's up to you.
I added that test and integrated into our repo; patch is attached.
>>> Would that be worth a one- or two-line comment?
>>
>> There's already the comment that it's padded out to avoid false
>> sharing; does it merit more than that?
>
> I guess that's sufficient, but I needed to remind myself that these
> were per-CPU data structures. Your call.
I'm going to leave that one as it is; in addition to the comment next
to the member, the block comment immediately above the structure also
spells that out:
/*
* DTrace Buffers
*
* Principal buffers, aggregation buffers, and speculative buffers are all
* managed with the dtrace_buffer structure. By default, this structure
* includes twin data buffers -- dtb_tomax and dtb_xamot -- that serve as the
* active and passive buffers, respectively. For speculative buffers,
* dtb_xamot will be NULL; for "ring" and "fill" buffers, dtb_xamot will point
* to a scratch buffer. For all buffer types, the dtrace_buffer structure is
* always allocated on a per-CPU basis; a single dtrace_buffer structure is
* never shared among CPUs. (That is, there is never true sharing of the
* dtrace_buffer structure; to prevent false sharing of the structure, it must
* always be aligned to the coherence granularity -- generally 64 bytes.)
*
...
Let me know if you have any other comments; otherwise, I'll assume
that you've moved on to reviewing unprivileged tick probes. ;)
- Bryan
dtrace-reap-testring.patch
Description: Binary data
_______________________________________________ dtrace-discuss mailing list dtrace-discuss@opensolaris.org
