From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754835AbdJIQlc (ORCPT ); Mon, 9 Oct 2017 12:41:32 -0400 Received: from mail.kernel.org ([198.145.29.99]:47242 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754637AbdJIQlb (ORCPT ); Mon, 9 Oct 2017 12:41:31 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7308F21877 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=goodmis.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=rostedt@goodmis.org Date: Mon, 9 Oct 2017 12:41:27 -0400 From: Steven Rostedt To: Jonathan Corbet Cc: Masami Hiramatsu , Kees Cook , Ingo Molnar , Linus Torvalds , Peter Zijlstra , Alexei Starovoitov , Ananth N Mavinakayanahalli , "Paul E . McKenney" , Thomas Gleixner , LKML , "H . Peter Anvin" , Anil S Keshavamurthy , "David S . Miller" , Ian McDonald , Vlad Yasevich , Stephen Hemminger Subject: Re: [RFC PATCH -tip 0/5] kprobes: Abolish jprobe APIs Message-ID: <20171009124127.616e1fc3@gandalf.local.home> In-Reply-To: <20171009103317.5701528f@lwn.net> References: <150724519527.5014.10207042218696587159.stgit@devbox> <20171005195808.2525a155@vmware.local.home> <20171006134959.e50c904ba6f302126cf4418c@kernel.org> <20171006113430.2c31561b@gandalf.local.home> <20171009093350.2680fcdf@lwn.net> <20171009122035.053ce63c@gandalf.local.home> <20171009103317.5701528f@lwn.net> X-Mailer: Claws Mail 3.14.0 (GTK+ 2.24.31; x86_64-pc-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 On Mon, 9 Oct 2017 10:33:17 -0600 Jonathan Corbet wrote: > On Mon, 9 Oct 2017 12:20:35 -0400 > Steven Rostedt wrote: > > > > > SAVE_REGS_IF_SUPPORTED - Similar to SAVE_REGS but the registering of a > > > > ftrace_ops on an architecture that does not support passing of regs > > > > will not fail with this flag set. But the callback must check if > > > > regs is NULL or not to determine if the architecture supports it. > > > > > > > > RECURSION_SAFE - By default, a wrapper is added around the callback to > > > > make sure that recursion of the function does not occur. That is > > > > if a function within the callback itself is also traced, ftrace > > > > > > s/within the/called by the/ > > > > I put in "within" because it is usually a function that is nested > > within a function called by the callback. This bug has come up with > > "gotchas", where some function that the callback calls has a path to a > > function that was unexpectedly traced. > > > > The issue hasn't been caused by a function being traced that was > > directly called by the callback. It is usually some deeper nested > > function. > > > > I don't want to limit it to just checking functions that the callback > > calls. Thoughts on how to stress this? > > "if a function that is called as a result of the callback's execution is > also traced" ? Sure, I'm not sure I could come up with a better way to say it. > > > > > IPMODIFY - Requires SAVE_REGS set. If the callback is to "hijack" the > > > > traced function (have another function called instead of the traced > > > > function), it requires setting this flag. This is what live kernel > > > > patches uses. Without this flag the pt_regs->ip can not be modified. > > > > Note, only one ftrace_ops with IPMODIFY set may be registered to > > > > any given function at a time. > > > > > > I assume this requires being able to get the regs too? > > > > Yes, this is why I stated "Requires SAVE_REGS" which would pass the > > regs to the callback. Should I rewrite that somehow? > > No, just ship me another cup of coffee and that one should be OK. Though > perhaps if you'd spelled out the flag completely I wouldn't have been so > dense :) OK OK, I'll extend the names. > > > > > If a glob is used to set the filter, to remove unwanted matches the > > > > ftrace_set_notrace() can also be used. > > > > > > > > int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf, > > > > int len, int reset); > > > > > > > > This takes the same parameters as ftrace_set_filter() but will add the > > > > functions it finds to not be traced. This doesn't remove them from the > > > > filter itself, but keeps them from being traced. If @reset is set, > > > > the filter is cleaded but the functions that match @buf will still not > > > > > > cleaded? :) > > > > Hmm, I'll have to be more descriptive. > > > > > > > > > be traced (the callback will not be called on those functions). > > > > > > So how do you clead the "notrace" list? > > > > With passing in reset non-zero. I'll add that. > > My confusion remains here. The text says that if reset is "set", then the > "notrace" list remains in place. So a non-zero "reset" value will remove > previous notrace entries, along with the filter itself? So if you wanted > to clear the notrace list entirely you would use buf="", reset=1? It would > be good to be explicit there. Ah I see what you mean. I'll try to be more explicit. A non-zero value for reset means to clear the notrace buffer before making modifications. In fact, I think the text is a bit more confusing, as I don't believe that this function even modifies the "set_filter" list, even if reset is set. And yes, to clear all functions in either the set_filter or set_notrace lists just pass in (&ops, NULL, 0, 1) I'll start working on this document and post a real patch. Thanks again for the review. -- Steve