Re: Revisiting DTrace syscall provider

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Wed, Feb 25, 2015 at 3:25 PM, Paul Goyette <paul%vps1.whooppee.com@localhost> wrote:
> On Wed, 25 Feb 2015, Ryota Ozaki wrote:
>
>> On Wed, Feb 25, 2015 at 2:38 PM, Paul Goyette <paul%vps1.whooppee.com@localhost>
>> wrote:
>>>
>>> On Wed, 25 Feb 2015, Paul Goyette wrote:
>>>
>>>> modules that provide sys-calls.  Without the module loaded, the syscall
>>>> entry point is known no-op; when the module tries to unload, all of its
>>>> syscalls are checked, and if any of them points to somewhere NOT THE
>>>> NOOP,
>>>> unload is prevented.
>>>
>>>
>>>
>>> Actually, the code checks to see if any those syscalls are currently
>>> active
>>> for any processes.
>>>
>>> IIRC, the relevant code is in routine syscall_disestablish() in file
>>> src/sys/kern_syscalls.c
>>
>>
>> Thank you for your suggestions! I'll check them.
>>
>> BTW, how about using moudle_hold(9) for the refcount?
>> Shouldn't we use it for the purpose?
>
>
> You could use module_hold().  But can you guarantee (for some value of
> guarantee) that module_release() will be called in all cases, including
> errors (ie, unexpected process exit).  :)

IIUC we only need to track dtrace_open and dtrace_close; the former
enables tracing and the latter disable it. Thought I must need more
investigations.

Anyway here is an updated patch that prevents a panic on moduload dtrace.
(This is only for dtrace.kmod. For systrace, we need other fixes.)

  ozaki-r

diff --git a/external/cddl/osnet/dev/dtrace/dtrace_modevent.c
b/external/cddl/osnet/dev/dtrace/dtrace_modevent.c
index 9df0cd1..30e6b0c 100644
--- a/external/cddl/osnet/dev/dtrace/dtrace_modevent.c
+++ b/external/cddl/osnet/dev/dtrace/dtrace_modevent.c
@@ -29,6 +29,7 @@ static int
 dtrace_modcmd(modcmd_t cmd, void *data)
 {
        int bmajor = -1, cmajor = -1;
+       int error;

        switch (cmd) {
        case MODULE_CMD_INIT:
@@ -36,8 +37,12 @@ dtrace_modcmd(modcmd_t cmd, void *data)
                return devsw_attach("dtrace", NULL, &bmajor,
                    &dtrace_cdevsw, &cmajor);
        case MODULE_CMD_FINI:
-               dtrace_unload();
+               error = dtrace_unload();
+               if (error != 0)
+                       return error;
                return devsw_detach(NULL, &dtrace_cdevsw);
+       case MODULE_CMD_AUTOUNLOAD:
+               return EBUSY;
        default:
                return ENOTTY;
        }
diff --git a/external/cddl/osnet/dev/dtrace/dtrace_unload.c
b/external/cddl/osnet/dev/dtrace/dtrace_unload.c
index 0d9f9fa..87b7a8e 100644
--- a/external/cddl/osnet/dev/dtrace/dtrace_unload.c
+++ b/external/cddl/osnet/dev/dtrace/dtrace_unload.c
@@ -37,9 +37,7 @@ dtrace_unload()
        mutex_enter(&dtrace_lock);
        mutex_enter(&cpu_lock);

-       ASSERT(dtrace_opens == 0);
-
-       if (dtrace_helpers > 0) {
+       if (dtrace_opens > 0 || dtrace_helpers > 0) {
                mutex_exit(&cpu_lock);
                mutex_exit(&dtrace_lock);