[illumos-Developer] Reaping enablings on defunct providers

[Adam Leventhal]

Hey Bryan,

Cool; looks good to me.

Adam

On Tue, Jul 12, 2011 at 10:44 PM, Bryan Cantrill <bryan at joyent.com> wrote:
> On Tue, Jul 12, 2011 at 5:25 PM, Adam Leventhal <ahl at 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
>



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

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