Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754584AbYKZSES (ORCPT ); Wed, 26 Nov 2008 13:04:18 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1752401AbYKZSEG (ORCPT ); Wed, 26 Nov 2008 13:04:06 -0500 Received: from smtp1.linux-foundation.org ([140.211.169.13]:45305 "EHLO smtp1.linux-foundation.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752256AbYKZSEF (ORCPT ); Wed, 26 Nov 2008 13:04:05 -0500 Date: Wed, 26 Nov 2008 10:02:55 -0800 From: Andrew Morton To: Steven Rostedt Cc: linux-kernel@vger.kernel.org, Ingo Molnar , Frederic Weisbecker , containers@lists.osdl.org, Sukadev Bhattiprolu , "Serge E. Hallyn" , "Eric W. Biederman" , Steven Rostedt , Harvey Harrison Subject: Re: [PATCH 2/5] ftrace: use code patching for ftrace graph tracer Message-Id: <20081126100255.d53f9347.akpm@linux-foundation.org> In-Reply-To: References: <20081126051622.134970943@goodmis.org> <20081126051709.774546196@goodmis.org> <20081125213546.ff4eddf4.akpm@linux-foundation.org> X-Mailer: Sylpheed 2.4.8 (GTK+ 2.12.5; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2269 Lines: 70 On Wed, 26 Nov 2008 11:46:58 -0500 (EST) Steven Rostedt wrote: > > > On Tue, 25 Nov 2008, Andrew Morton wrote: > > > On Wed, 26 Nov 2008 00:16:24 -0500 Steven Rostedt wrote: > > > > > From: Steven Rostedt > > > > > > Impact: more efficient code for ftrace graph tracer > > > > > > This patch uses the dynamic patching, when available, to patch > > > the function graph code into the kernel. > > > > > > This patch will ease the way for letting both function tracing > > > and function graph tracing run together. > > > > > > ... > > > > > > +static int ftrace_mod_jmp(unsigned long ip, > > > + int old_offset, int new_offset) > > > +{ > > > + unsigned char code[MCOUNT_INSN_SIZE]; > > > + > > > + if (probe_kernel_read(code, (void *)ip, MCOUNT_INSN_SIZE)) > > > + return -EFAULT; > > > + > > > + if (code[0] != 0xe9 || old_offset != *(int *)(&code[1])) > > > > erk. I suspect that there's a nicer way of doing this amongst our > > forest of get_unaligned_foo() interfaces. Harvey will know. > > Hmm, I may be able to make a struct out of code. > > struct { > unsigned char op; > unsigned int offset; > } code __attribute__((packed)); > > Would that look better? nah, let's do something more generic for this. > > > > > + return -EINVAL; > > > + > > > + *(int *)(&code[1]) = new_offset; > > > > Might be able to use put_unaligned_foo() here. > > > > The problem is that these functions use sizeof(*ptr) to work out what > > to do, so a cast is still needed. A get_unaligned32(ptr) would be > > nice. One which takes a void* and assumes CPU ordering. > > Is there a correctness concern here? This is arch specific code, so I'm > not worried about other archs. No, the code is OK as-is. It's just that "read a word from an [maybe-]unaligned address" is such a common operation that there should be a nice clean simple function to do it, rather than doing open-coded (and different) weird C tricks at each and\ every site. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/