From owner-svn-src-head@FreeBSD.ORG Mon May 11 01:55:43 2015 Return-Path: Delivered-To: svn-src-head@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 31B2480; Mon, 11 May 2015 01:55:43 +0000 (UTC) Received: from vps1.elischer.org (vps1.elischer.org [204.109.63.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "vps1.elischer.org", Issuer "CA Cert Signing Authority" (not verified)) by mx1.freebsd.org (Postfix) with ESMTPS id F1F92173E; Mon, 11 May 2015 01:55:42 +0000 (UTC) Received: from Julian-MBP3.local (ppp121-45-241-118.lns20.per4.internode.on.net [121.45.241.118]) (authenticated bits=0) by vps1.elischer.org (8.14.9/8.14.9) with ESMTP id t4B1tU5M062878 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Sun, 10 May 2015 18:55:33 -0700 (PDT) (envelope-from julian@freebsd.org) Message-ID: <55500C0D.7090802@freebsd.org> Date: Mon, 11 May 2015 09:55:25 +0800 From: Julian Elischer User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.10; rv:31.0) Gecko/20100101 Thunderbird/31.6.0 MIME-Version: 1.0 To: Mark Johnston , src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r282744 - in head/sys/cddl/dev/dtrace: amd64 i386 References: <201505102227.t4AMRn1g007294@svn.freebsd.org> In-Reply-To: <201505102227.t4AMRn1g007294@svn.freebsd.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 11 May 2015 01:55:43 -0000 On 5/11/15 6:27 AM, Mark Johnston wrote: > Author: markj > Date: Sun May 10 22:27:48 2015 > New Revision: 282744 > URL: https://svnweb.freebsd.org/changeset/base/282744 > > Log: > Remove some commented-out upstream code for handling traps from usermode > DTrace probes. This handling is already done in trap() on i386 and amd64. I take issue with the implied assertion that upstream code that is not needed for FreeBSD should be removed. Commented out upstream code makes it easier to merge in changes from upstream. it allows patches to be applied and for a lot less worry on the part of the merger. If the code is not there, and there is an upstream change to it, one has to ask "is there equivalent code somewhere else that needs to be patched instead?" But if there is just a #ifndef __FreeBSD__ /* this code is not needed in FreeBSD because ...*/ blah #endif /*__FreeBSD__ this code is not needed in FreeBSD */ then the merger has a much easier task. I've just gone through large code merge (where FreeBSD was "upstream") and the cases there the original FreebSD code was still present but #Ifdef'd out were SO MUCH easier to handle. > > Modified: > head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c > head/sys/cddl/dev/dtrace/i386/dtrace_subr.c > > Modified: head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c > ============================================================================== > --- head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c Sun May 10 22:21:00 2015 (r282743) > +++ head/sys/cddl/dev/dtrace/amd64/dtrace_subr.c Sun May 10 22:27:48 2015 (r282744) > @@ -142,114 +142,6 @@ dtrace_sync(void) > } > > #ifdef notyet > -int (*dtrace_pid_probe_ptr)(struct regs *); > -int (*dtrace_return_probe_ptr)(struct regs *); > - > -void > -dtrace_user_probe(struct regs *rp, caddr_t addr, processorid_t cpuid) > -{ > - krwlock_t *rwp; > - proc_t *p = curproc; > - extern void trap(struct regs *, caddr_t, processorid_t); > - > - if (USERMODE(rp->r_cs) || (rp->r_ps & PS_VM)) { > - if (curthread->t_cred != p->p_cred) { > - cred_t *oldcred = curthread->t_cred; > - /* > - * DTrace accesses t_cred in probe context. t_cred > - * must always be either NULL, or point to a valid, > - * allocated cred structure. > - */ > - curthread->t_cred = crgetcred(); > - crfree(oldcred); > - } > - } > - > - if (rp->r_trapno == T_DTRACE_RET) { > - uint8_t step = curthread->t_dtrace_step; > - uint8_t ret = curthread->t_dtrace_ret; > - uintptr_t npc = curthread->t_dtrace_npc; > - > - if (curthread->t_dtrace_ast) { > - aston(curthread); > - curthread->t_sig_check = 1; > - } > - > - /* > - * Clear all user tracing flags. > - */ > - curthread->t_dtrace_ft = 0; > - > - /* > - * If we weren't expecting to take a return probe trap, kill > - * the process as though it had just executed an unassigned > - * trap instruction. > - */ > - if (step == 0) { > - tsignal(curthread, SIGILL); > - return; > - } > - > - /* > - * If we hit this trap unrelated to a return probe, we're > - * just here to reset the AST flag since we deferred a signal > - * until after we logically single-stepped the instruction we > - * copied out. > - */ > - if (ret == 0) { > - rp->r_pc = npc; > - return; > - } > - > - /* > - * We need to wait until after we've called the > - * dtrace_return_probe_ptr function pointer to set %pc. > - */ > - rwp = &CPU->cpu_ft_lock; > - rw_enter(rwp, RW_READER); > - if (dtrace_return_probe_ptr != NULL) > - (void) (*dtrace_return_probe_ptr)(rp); > - rw_exit(rwp); > - rp->r_pc = npc; > - > - } else if (rp->r_trapno == T_BPTFLT) { > - uint8_t instr; > - rwp = &CPU->cpu_ft_lock; > - > - /* > - * The DTrace fasttrap provider uses the breakpoint trap > - * (int 3). We let DTrace take the first crack at handling > - * this trap; if it's not a probe that DTrace knowns about, > - * we call into the trap() routine to handle it like a > - * breakpoint placed by a conventional debugger. > - */ > - rw_enter(rwp, RW_READER); > - if (dtrace_pid_probe_ptr != NULL && > - (*dtrace_pid_probe_ptr)(rp) == 0) { > - rw_exit(rwp); > - return; > - } > - rw_exit(rwp); > - > - /* > - * If the instruction that caused the breakpoint trap doesn't > - * look like an int 3 anymore, it may be that this tracepoint > - * was removed just after the user thread executed it. In > - * that case, return to user land to retry the instuction. > - */ > - if (fuword8((void *)(rp->r_pc - 1), &instr) == 0 && > - instr != FASTTRAP_INSTR) { > - rp->r_pc--; > - return; > - } > - > - trap(rp, addr, cpuid); > - > - } else { > - trap(rp, addr, cpuid); > - } > -} > - > void > dtrace_safe_synchronous_signal(void) > { > > Modified: head/sys/cddl/dev/dtrace/i386/dtrace_subr.c > ============================================================================== > --- head/sys/cddl/dev/dtrace/i386/dtrace_subr.c Sun May 10 22:21:00 2015 (r282743) > +++ head/sys/cddl/dev/dtrace/i386/dtrace_subr.c Sun May 10 22:27:48 2015 (r282744) > @@ -143,122 +143,6 @@ dtrace_sync(void) > } > > #ifdef notyet > -int (*dtrace_fasttrap_probe_ptr)(struct regs *); > -int (*dtrace_pid_probe_ptr)(struct regs *); > -int (*dtrace_return_probe_ptr)(struct regs *); > - > -void > -dtrace_user_probe(struct regs *rp, caddr_t addr, processorid_t cpuid) > -{ > - krwlock_t *rwp; > - proc_t *p = curproc; > - extern void trap(struct regs *, caddr_t, processorid_t); > - > - if (USERMODE(rp->r_cs) || (rp->r_ps & PS_VM)) { > - if (curthread->t_cred != p->p_cred) { > - cred_t *oldcred = curthread->t_cred; > - /* > - * DTrace accesses t_cred in probe context. t_cred > - * must always be either NULL, or point to a valid, > - * allocated cred structure. > - */ > - curthread->t_cred = crgetcred(); > - crfree(oldcred); > - } > - } > - > - if (rp->r_trapno == T_DTRACE_RET) { > - uint8_t step = curthread->t_dtrace_step; > - uint8_t ret = curthread->t_dtrace_ret; > - uintptr_t npc = curthread->t_dtrace_npc; > - > - if (curthread->t_dtrace_ast) { > - aston(curthread); > - curthread->t_sig_check = 1; > - } > - > - /* > - * Clear all user tracing flags. > - */ > - curthread->t_dtrace_ft = 0; > - > - /* > - * If we weren't expecting to take a return probe trap, kill > - * the process as though it had just executed an unassigned > - * trap instruction. > - */ > - if (step == 0) { > - tsignal(curthread, SIGILL); > - return; > - } > - > - /* > - * If we hit this trap unrelated to a return probe, we're > - * just here to reset the AST flag since we deferred a signal > - * until after we logically single-stepped the instruction we > - * copied out. > - */ > - if (ret == 0) { > - rp->r_pc = npc; > - return; > - } > - > - /* > - * We need to wait until after we've called the > - * dtrace_return_probe_ptr function pointer to set %pc. > - */ > - rwp = &CPU->cpu_ft_lock; > - rw_enter(rwp, RW_READER); > - if (dtrace_return_probe_ptr != NULL) > - (void) (*dtrace_return_probe_ptr)(rp); > - rw_exit(rwp); > - rp->r_pc = npc; > - > - } else if (rp->r_trapno == T_DTRACE_PROBE) { > - rwp = &CPU->cpu_ft_lock; > - rw_enter(rwp, RW_READER); > - if (dtrace_fasttrap_probe_ptr != NULL) > - (void) (*dtrace_fasttrap_probe_ptr)(rp); > - rw_exit(rwp); > - > - } else if (rp->r_trapno == T_BPTFLT) { > - uint8_t instr; > - rwp = &CPU->cpu_ft_lock; > - > - /* > - * The DTrace fasttrap provider uses the breakpoint trap > - * (int 3). We let DTrace take the first crack at handling > - * this trap; if it's not a probe that DTrace knowns about, > - * we call into the trap() routine to handle it like a > - * breakpoint placed by a conventional debugger. > - */ > - rw_enter(rwp, RW_READER); > - if (dtrace_pid_probe_ptr != NULL && > - (*dtrace_pid_probe_ptr)(rp) == 0) { > - rw_exit(rwp); > - return; > - } > - rw_exit(rwp); > - > - /* > - * If the instruction that caused the breakpoint trap doesn't > - * look like an int 3 anymore, it may be that this tracepoint > - * was removed just after the user thread executed it. In > - * that case, return to user land to retry the instuction. > - */ > - if (fuword8((void *)(rp->r_pc - 1), &instr) == 0 && > - instr != FASTTRAP_INSTR) { > - rp->r_pc--; > - return; > - } > - > - trap(rp, addr, cpuid); > - > - } else { > - trap(rp, addr, cpuid); > - } > -} > - > void > dtrace_safe_synchronous_signal(void) > { > >