From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754044AbYHSOrZ (ORCPT ); Tue, 19 Aug 2008 10:47:25 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752040AbYHSOrP (ORCPT ); Tue, 19 Aug 2008 10:47:15 -0400 Received: from tomts13-srv.bellnexxia.net ([209.226.175.34]:59079 "EHLO tomts13-srv.bellnexxia.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752402AbYHSOrO (ORCPT ); Tue, 19 Aug 2008 10:47:14 -0400 X-IronPort-Anti-Spam-Filtered: true X-IronPort-Anti-Spam-Result: AtIFANV4qkhMRKxB/2dsb2JhbACBYrNAgVg Date: Tue, 19 Aug 2008 10:42:11 -0400 From: Mathieu Desnoyers To: Eran Liberty Cc: Steven Rostedt , Steven Rostedt , linux-kernel@vger.kernel.org, "Paul E. McKenney" , linuxppc-dev@ozlabs.org Subject: Re: ftrace introduces instability into kernel 2.6.27(-rc2,-rc3) Message-ID: <20080819144211.GB21929@Krystal> References: <48591941.4070408@extricom.com> <48A92E15.2080709@extricom.com> <48A9901B.1080900@redhat.com> <48A9BEA3.10906@extricom.com> <48AAB7FF.4070502@extricom.com> <20080819130533.GA18611@Krystal> <48AAD702.10700@extricom.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline In-Reply-To: <48AAD702.10700@extricom.com> X-Editor: vi X-Info: http://krystal.dyndns.org:8080 X-Operating-System: Linux/2.6.21.3-grsec (i686) X-Uptime: 10:33:38 up 75 days, 19:14, 7 users, load average: 0.29, 0.47, 0.69 User-Agent: Mutt/1.5.16 (2007-06-11) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org * Eran Liberty (liberty@extricom.com) wrote: > Mathieu Desnoyers wrote: >> Can you also give us >> >> objdump -S --start-address=0xC00BB724 vmlinux | head 20 >> >> ? >> >> Then we could compare the result with the OOPS instruction dump : >> >> 7c0802a6 bf61000c 3f60c038 7c3f0b78 90010024 7c7c1b78 7c9d2378 83db32a0 >> 73c00001 7f83e378 7fa4eb78 4082002f <00000000> 2f830000 409e0030 801b32a0 >> >> Mathieu >> >> > > to give you more context I have run: > > powerpc-linux-gnu-objdump -S --start-address=0xC00BB700 vmlinux | head -n > 60 > > the discrepancy starts at address: > c00bb720: 40 82 00 30 <=> 40 82 00 2f > c00bb724: 4b ff fe 61 <=> 00 00 00 00 > Ah ! I think I see what could be wrong : First we have : static unsigned int ftrace_nop = 0x60000000; We probably replace the original function call by this nop. Then we do : notrace unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr) { static unsigned int op; /* * It would be nice to just use create_function_call, but that will * update the code itself. Here we need to just return the * instruction that is going to be modified, without modifying the * code. */ addr = GET_ADDR(addr); /* Set to "bl addr" */ op = 0x48000001 | (ftrace_calc_offset(ip, addr) & 0x03fffffc); /* * No locking needed, this must be called via kstop_machine * which in essence is like running on a uniprocessor machine. */ return (unsigned char *)&op; } And I guess we must be doing a 0x48000001 | 0x0; or something ? Also, we have to consider that POWERPC 64 functions are : /* PowerPC64's functions are data that points to the functions */ And this does not seem to hold for ppc32. Therefore, it is strange to me that the same code is used for the update.. are we updating the correct site ? Mathieu > vmlinux: file format elf32-powerpc > > Disassembly of section .text: > > c00bb700 : > * d_lookup() is protected against the concurrent renames in some unrelated > * directory using the seqlockt_t rename_lock. > */ > > struct dentry * d_lookup(struct dentry * parent, struct qstr * name) > { > c00bb700: 7c 3f 0b 78 mr r31,r1 > c00bb704: 90 01 00 24 stw r0,36(r1) > c00bb708: 7c 7c 1b 78 mr r28,r3 > c00bb70c: 7c 9d 23 78 mr r29,r4 > c00bb710: 83 db 32 a0 lwz r30,12960(r27) > { > unsigned ret; > > repeat: > ret = sl->sequence; > smp_rmb(); > c00bb714: 73 c0 00 01 andi. r0,r30,1 > struct dentry * dentry = NULL; > unsigned long seq; > > do { > seq = read_seqbegin(&rename_lock); > dentry = __d_lookup(parent, name); > c00bb718: 7f 83 e3 78 mr r3,r28 > c00bb71c: 7f a4 eb 78 mr r4,r29 > if (unlikely(ret & 1)) { > c00bb720: 40 82 00 30 bne- c00bb750 > c00bb724: 4b ff fe 61 bl c00bb584 <__d_lookup> > if (dentry) > c00bb728: 2f 83 00 00 cmpwi cr7,r3,0 > c00bb72c: 40 9e 00 30 bne- cr7,c00bb75c > * > * If sequence value changed then writer changed data while in section. > */ > static __always_inline int read_seqretry(const seqlock_t *sl, unsigned > start) > { > smp_rmb(); > c00bb730: 80 1b 32 a0 lwz r0,12960(r27) > break; > } while (read_seqretry(&rename_lock, seq)); > c00bb734: 7f 80 f0 00 cmpw cr7,r0,r30 > c00bb738: 41 9e 00 24 beq- cr7,c00bb75c > /* Start of read calculation -- fetch last complete writer token */ > static __always_inline unsigned read_seqbegin(const seqlock_t *sl) > { > unsigned ret; > > repeat: > c00bb73c: 7c 1e 03 78 mr r30,r0 > ret = sl->sequence; > smp_rmb(); > c00bb740: 73 c0 00 01 andi. r0,r30,1 > struct dentry * dentry = NULL; > -- Mathieu Desnoyers OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68