Re: [patch] Userland DTrace

On Fri, Feb 08, 2013 at 04:04:38PM +0000, 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.

I've run into this too. It can happen even when I'm not using DTrace
since fasttrap.ko is always loaded on my system. The problem is that
fasttrap_exec_exit() is called every time a process exits in this case;
the caller acquires dtrace_lock, and the panic occurs when a callout
thread tries to acquire the lock at the same time.

> 
> 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

Another panic can occur with an INVARIANTS kernel if a DTrace victim
process forks. I've supplied a patch which fixes this for me here:
http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/171360

> 
> 
> 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)
> +                       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;
>  
> 
> -- 
> Sorry for the following...
> The information contained in this message is confidential and intended for the addressee only. If you have received this message in error, or there are any problems with its content, please contact the sender. 
> 
> iCritical is a trading name of Critical Software Ltd. Registered in England: 04909220.
> Registered Office: IC2, Keele Science Park, Keele, Staffordshire, ST5 5NH.
> 
> This message has been scanned for security threats by iCritical. www.icritical.com
> 
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"