From owner-freebsd-sparc64@FreeBSD.ORG Thu Feb 16 12:48:32 2006 Return-Path: X-Original-To: freebsd-sparc64@freebsd.org Delivered-To: freebsd-sparc64@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 95FF616A420; Thu, 16 Feb 2006 12:48:32 +0000 (GMT) (envelope-from marius@newtrinity.zeist.de) Received: from newtrinity.zeist.de (newtrinity.zeist.de [217.24.217.8]) by mx1.FreeBSD.org (Postfix) with ESMTP id E988943D46; Thu, 16 Feb 2006 12:48:31 +0000 (GMT) (envelope-from marius@newtrinity.zeist.de) Received: from newtrinity.zeist.de (localhost [127.0.0.1]) by newtrinity.zeist.de (8.12.11/8.12.11/ZEIST.DE) with ESMTP id k1GCmUr1058287; Thu, 16 Feb 2006 13:48:30 +0100 (CET) (envelope-from marius@newtrinity.zeist.de) Received: (from marius@localhost) by newtrinity.zeist.de (8.12.11/8.12.10/Submit) id k1GCmOQK058284; Thu, 16 Feb 2006 13:48:24 +0100 (CET) (envelope-from marius) Date: Thu, 16 Feb 2006 13:48:24 +0100 From: Marius Strobl To: Antoine Brodin , jhb@freebsd.org Message-ID: <20060216134823.S53619@newtrinity.zeist.de> References: <200602131150.k1DBo6S1074438@freefall.freebsd.org> <200602131223.51561.jhb@freebsd.org> <20060213193613.547d1b8f.antoine.brodin@laposte.net> <200602131430.11228.jhb@freebsd.org> <20060213213719.7767921e.antoine.brodin@laposte.net> <20060214094744.A81690@newtrinity.zeist.de> <20060214205432.38121641.antoine.brodin@laposte.net> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="uZ3hkaAS1mZxFaxD" Content-Disposition: inline User-Agent: Mutt/1.2.5.1i In-Reply-To: <20060214205432.38121641.antoine.brodin@laposte.net>; from antoine.brodin@laposte.net on Tue, Feb 14, 2006 at 08:54:32PM +0100 X-AntiVirus-modified: yes X-AntiVirus: checked by AntiVir Milter (version: 1.1.2-1; AVE: 6.33.0.31; VDF: 6.33.0.240; host: newtrinity.zeist.de) Cc: freebsd-sparc64@freebsd.org Subject: Re: sparc64/93226: DEBUG_LOCKS (really stack_save()) causes panics on sparc64 X-BeenThere: freebsd-sparc64@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the Sparc List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 16 Feb 2006 12:48:32 -0000 --uZ3hkaAS1mZxFaxD Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Feb 14, 2006 at 08:54:32PM +0100, Antoine Brodin wrote: > Marius Strobl wrote: > > On Mon, Feb 13, 2006 at 09:37:19PM +0100, Antoine Brodin wrote: > > > John Baldwin wrote: > > > > On Monday 13 February 2006 13:36, Antoine Brodin wrote: > > > > > John Baldwin wrote: > > > > > > If there are kernel functions before the assembly ones (dependent on link > > > > > > order) then this would wrongly bail when it hit those. I think you need > > > > > > to do what the ddb stack tracing code does which is to lookup the symbol > > > > > > name and do a bcmp() on the first 4 chars to recognize trapframes. > > > > > > > > > > I ran objdump -d on a sparc64 kernel and it looks like tl0_* and tl1_* > > > > > are always at the beginning of the code, there is some kind of magic. > > > > > > > > magic aside, it would be best to use the same algorithm in both places IMO. > > > > It would also be a lot more intuitive to other folks later on. > > > > > > There are 2 reasons why I didn't use db_search_symbol() and > > > db_symbol_values() : > > > > > > - first they aren't reentrant, they use a global variable > > > db_last_symtab and they can panic if a thread sets db_last_symtab to 0 > > > while another one is using it. I found this in my mail archive : > > > %%% > > > Stopped at X_db_symbol_values+0x18: cmpl $0,0xc(%eax) > > > db> trace > > > Tracing pid 34983 tid 100093 td 0xc2e9c640 > > > X_db_symbol_values(0,c0738214,e84859f4,e84859c4,7c) at X_db_symbol_values+0x18 > > > db_symbol_values(c0738214,e84859f4,0,c89d19c8,0) at db_symbol_values+0x40 > > > %%% > > > It can be fixed easily but I don't have the fix anymore. > > > You can use linker_ddb_search_symbol() and linker_ddb_symbol_values() > > > too that are safer. > > > > > > - the second reason is performance. if you replace > > > CTRSTACK(KTR_LOCK, &stack, 0, 1); > > > by > > > CTRSTACK(KTR_LOCK, &stack, 0, 0); > > > in kern_lock.c, i.e. if you print the symbol name in the ktr traces, you > > > will notice that your box is extremely slow. (you type ls, you wait 4 or 5 > > > seconds and you have the result) > > > > > > > Can't we just use what's done in support.S and add two dummy symbols > > to mark the begin and end of the asm functions in question? > > There's already a tl0_base symbol. You can add a tl0_end (or tl1_end ?) > symbol between tl1_intr and fork_trampoline. > Ok, how about the attached patch? It uses two pairs of dummy symbols in exception.S to determine in stack_save() whether it was one of the tl0_*() or tl1_*() asm functions; one pair for those in the .trap section that is "magically" placed at the beginning of the .text section via the linker script and the other pair for those in the regular .text section. That way we don't rely on the location of these functions in the kernel and don't have the performance penalty of *search_symbol()/*symbol_values(). For consistency db_backtrace() is changed to also use the new markers instead of bcmp()'ing with the symbol names. Marius -- This mail was scanned by AntiVir Milter. This product is licensed for non-commercial use. See www.antivir.de for details. --uZ3hkaAS1mZxFaxD Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="db_tl.diff" Index: exception.S =================================================================== RCS file: /mnt/futile/usr/data/bsd/cvs/fbsd/src/sys/sparc64/sparc64/exception.S,v retrieving revision 1.71 diff -u -r1.71 exception.S --- exception.S 16 Jan 2006 01:35:25 -0000 1.71 +++ exception.S 16 Feb 2006 11:59:50 -0000 @@ -166,6 +166,10 @@ ldx [ASP_REG + 0], %g1 ; \ inc 16, ASP_REG + .globl tl_text_begin +tl_text_begin: + nop + ENTRY(tl1_kstack_fault) rdpr %tl, %g1 1: cmp %g1, 2 @@ -1818,6 +1822,10 @@ .endm .sect .trap + .globl tl_trap_begin +tl_trap_begin: + nop + .align 0x8000 .globl tl0_base @@ -2015,6 +2023,10 @@ tl1_gen T_RSTRWP_VIRT ! 0x303 tl1_reserved 252 ! 0x304-0x3ff + .globl tl_trap_end +tl_trap_end: + nop + /* * User trap entry point. * @@ -2901,6 +2913,10 @@ retry END(tl1_intr) + .globl tl_text_end +tl_text_end: + nop + /* * Freshly forked processes come here when switched to for the first time. * The arguments to fork_exit() have been setup in the locals, we must move Index: db_trace.c =================================================================== RCS file: /mnt/futile/usr/data/bsd/cvs/fbsd/src/sys/sparc64/sparc64/db_trace.c,v retrieving revision 1.24 diff -u -r1.24 db_trace.c --- db_trace.c 3 Aug 2005 04:27:39 -0000 1.24 +++ db_trace.c 16 Feb 2006 11:51:27 -0000 @@ -49,6 +49,11 @@ #include #include +extern char tl_trap_begin[]; +extern char tl_trap_end[]; +extern char tl_text_begin[]; +extern char tl_text_end[]; + #define INKERNEL(va) \ ((va) >= VM_MIN_KERNEL_ADDRESS && (va) <= VM_MAX_KERNEL_ADDRESS) @@ -259,8 +264,10 @@ name = "(null)"; fp = (struct frame *)(db_get_value((db_addr_t)&fp->fr_fp, sizeof(fp->fr_fp), FALSE) + SPOFF); - if (bcmp(name, "tl0_", 4) == 0 || - bcmp(name, "tl1_", 4) == 0) { + if ((value > (u_long)tl_trap_begin && + value < (u_long)tl_trap_end) || + (value > (u_long)tl_text_begin && + value < (u_long)tl_text_end)) { tf = (struct trapframe *)(fp + 1); npc = db_get_value((db_addr_t)&tf->tf_tpc, sizeof(tf->tf_tpc), FALSE); @@ -307,6 +314,12 @@ callpc = fp->fr_pc; if (!INKERNEL(callpc)) break; + /* Don't bother traversing trap frames. */ + if ((callpc > (u_long)tl_trap_begin && + callpc < (u_long)tl_trap_end) || + (callpc > (u_long)tl_text_begin && + callpc < (u_long)tl_text_end)) + break; if (stack_put(st, callpc) == -1) break; fp = (struct frame *)(fp->fr_fp + SPOFF); --uZ3hkaAS1mZxFaxD--