Re: [patch] Userland DTrace

On 2/8/13 8:04 PM, Matt Burke wrote:
> I've been spending some time trying to get the fasttrap provider to work
> on FreeBSD without panicing. I believe I have succeeded, at least to the
> point where it's no longer panicing.
> 
> There were two panic causes. The first was
> http://www.freebsd.org/cgi/query-pr.cgi?pr=165541 - the FreeBSD port of
> fasttrap.c caused ftp_rcount to be left >0. To fix this I've got rid of
> the early return and reverted to the opensolaris way.
> 
> A second panic then showed up intermittently when fasttrap_pid_cleanup_cb
> was run while something in userland had locks. Using sx_try_xlock calls
> has stopped the panics and shouldn't affect operation AFAICT.
> 
> This is against r246454.
> 
> 
> Although this has fixed the panics for me, I'm finding a lot of stuff just
> isn't actually working, with dtrace and the traced process just chewing
> CPU. Truss on the dtrace shows a heck of a lot of ptrace() calls and I
> have no idea what the target is doing... CPU time is split 2:1
> dtrace:target
> 

Great!  This fixes panics for me too, but I still cannot get something
useful tracing and after detaching by ctrl+c my programs still segfaults.

Please look at one style comment below.

> 
> Also noteworthy is the LOR on the first time you try to use the fasttrap
> provider: http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/165479
> 
> The lock order there seems right, so I'm guessing "something else" must
> have done it wrong first? How can I find out what the "something else"
> is?
> 
> 
> Thanks
> 
> 
> --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
> +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/dtrace.c
> _at__at_ -7536,9 +7536,23 _at__at_ dtrace_unregister(dtrace_provider_id_t id)
>                         return (EBUSY);
>                 }
>         } else {
> +#if defined(sun)
>                 mutex_enter(&dtrace_provider_lock);
>                 mutex_enter(&mod_lock);
>                 mutex_enter(&dtrace_lock);
> +#else
> +               if (sx_try_xlock(&dtrace_provider_lock) == 0)

s/sx_try_xlock/mutex_tryenter/

Look at sys/cddl/compat/opensolaris/sys/mutex.h

> +                       return (EBUSY);
> +               if (sx_try_xlock(&mod_lock) == 0) {
> +                       mutex_exit(&dtrace_provider_lock);
> +                       return (EBUSY);
> +               }
> +               if (sx_try_xlock(&dtrace_lock) == 0) {
> +                       mutex_exit(&mod_lock);
> +                       mutex_exit(&dtrace_provider_lock);
> +                       return (EBUSY);
> +               }
> +#endif
>         }
>  
>         /*
> --- a/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
> +++ b/sys/cddl/contrib/opensolaris/uts/common/dtrace/fasttrap.c
> _at__at_ -1116,23 +1116,28 _at__at_ fasttrap_pid_disable(void *arg, dtrace_id_t id, void *parg)
>  
>         ASSERT(id == probe->ftp_id);
>  
> -       mutex_enter(&provider->ftp_mtx);
> -
>         /*
>          * We won't be able to acquire a /proc-esque lock on the process
>          * iff the process is dead and gone. In this case, we rely on the
>          * provider lock as a point of mutual exclusion to prevent other
>          * DTrace consumers from disabling this probe.
>          */
> -       if ((p = pfind(probe->ftp_pid)) == NULL) {
> -               mutex_exit(&provider->ftp_mtx);
> -               return;
> +
> +#if defined(sun)
> +       if ((p = sprlock(probe->ftp_pid)) != NULL) {
> +               ASSERT(!(p->p_flag & SVFORK));
> +               mutex_exit(&p->p_lock);
> +       }
> +#else
> +       if ((p = pfind(probe->ftp_pid)) != NULL) {
> +               _PHOLD(p);
> +               PROC_UNLOCK(p);
>         }
> -#ifdef __FreeBSD__
> -       _PHOLD(p);
> -       PROC_UNLOCK(p);
>  #endif
>  
> +       mutex_enter(&provider->ftp_mtx);
> +
> +
>         /*
>          * Disable all the associated tracepoints (for fully enabled probes).
>          */
> _at__at_ -1154,6 +1159,13 _at__at_ fasttrap_pid_disable(void *arg, dtrace_id_t id, void *parg)
>                 if (provider->ftp_retired && !provider->ftp_marked)
>                         whack = provider->ftp_marked = 1;
>                 mutex_exit(&provider->ftp_mtx);
> +
> +#if defined(sun)
> +               mutex_enter(&p->p_lock);
> +               sprunlock(p);
> +#else
> +               PRELE(p);
> +#endif
>         } else {
>                 /*
>                  * If the process is dead, we're just waiting for the
> _at__at_ -1167,9 +1179,6 _at__at_ fasttrap_pid_disable(void *arg, dtrace_id_t id, void *parg)
>         if (whack)
>                 fasttrap_pid_cleanup();
>  
> -#ifdef __FreeBSD__
> -       PRELE(p);
> -#endif
>         if (!probe->ftp_enabled)
>                 return;
>  
> 


-- 
Andrey Zonov