From patchwork Fri May 25 23:20:36 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joel Fernandes X-Patchwork-Id: 10428697 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 50936601D5 for ; Fri, 25 May 2018 23:20:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 3AA192982A for ; Fri, 25 May 2018 23:20:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 2AC6B2982C; Fri, 25 May 2018 23:20:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 6EFF72982A for ; Fri, 25 May 2018 23:20:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030682AbeEYXUk (ORCPT ); Fri, 25 May 2018 19:20:40 -0400 Received: from mail-pf0-f195.google.com ([209.85.192.195]:36614 "EHLO mail-pf0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030538AbeEYXUi (ORCPT ); Fri, 25 May 2018 19:20:38 -0400 Received: by mail-pf0-f195.google.com with SMTP id w129-v6so3239751pfd.3 for ; Fri, 25 May 2018 16:20:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=gF8HjAjwaTtxyIyYCc4D5++3HgxRqgKbhk1y/EF4uHM=; b=oHPfQYuwAMfz5X0GOin3fJG5UsS5fhtoXz3Dy+0Zv6sYqvvpmTM2EcGOyNyTyNBqsp vx/7fKyzqZutOlOoeVszFqYbjxiqYuav0yRxx2s8q2Jm9ph0Tjza1rUkXRBCCGMNufdQ 60buMHQyJb3KfTaQdzKmG8N3+gaj70NiQxKVc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=gF8HjAjwaTtxyIyYCc4D5++3HgxRqgKbhk1y/EF4uHM=; b=ObvidISIICthxhHyj2SnOvXEsIAvPU7slDMgzYldhO+v7aGr5HlyyEw004D5wmoEJt xzh09xCETExSBmSU9bMdN5pM2anF5zq2J5I1BVVNsC6wTakOMI+wRYZmI9iloLKmC3Cq 2Jyn7TjllpfVPCg2Qw7oz14SA9Nc0c5xvANdVgTaBG8oMlEOHUieFQDIMvuR3FyDOiKn NyzLFWy1BOlRtHOdoVEer6qgTPLyakZ79cLadnB8BAM04Ddqr9i8ZhM9c5rCaSdk3Eea n0xE+DUwyPiOD+8JF4upkpEFQ0vq1lHhvVWeIN/8j7edTxY/rW0IGW4o0qkRgkXnhmPG k/rA== X-Gm-Message-State: ALKqPwfjUKFaEcfjsQblHsQsTAFwy/Bm2sq3hvVRD4lFryWY+j4RWEJo N721PCTN2gDyjsjCqN0CKs4DGA== X-Google-Smtp-Source: ADUXVKIbgPxh8nHhsi9F4lHl/3xVirDnsrWNDkF+WDJQD8are7xxwssL70v2DQpJosL08JJAMpg/cA== X-Received: by 2002:a62:d8c1:: with SMTP id e184-v6mr133957pfg.161.1527290438116; Fri, 25 May 2018 16:20:38 -0700 (PDT) Received: from localhost ([2620:0:1000:1600:3122:ea9c:d178:eb]) by smtp.gmail.com with ESMTPSA id e10-v6sm48318772pfb.136.2018.05.25.16.20.37 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Fri, 25 May 2018 16:20:37 -0700 (PDT) Date: Fri, 25 May 2018 16:20:36 -0700 From: Joel Fernandes To: Namhyung Kim Cc: Joel Fernandes , linux-kernel@vger.kernel.org, kernel-team@android.com, Boqun Feng , Byungchul Park , Erick Reyes , Ingo Molnar , Julia Cartwright , linux-kselftest@vger.kernel.org, Masami Hiramatsu , Mathieu Desnoyers , Paul McKenney , Peter Zijlstra , Shuah Khan , Steven Rostedt , Thomas Glexiner , Todd Kjos , Tom Zanussi , kernel-team@lge.com Subject: Re: [PATCH v7 6/8] tracing: Centralize preemptirq tracepoints and unify their usage Message-ID: <20180525232036.GA252027@joelaf.mtv.corp.google.com> References: <20180524012157.181277-1-joel@joelfernandes.org> <20180524012157.181277-7-joel@joelfernandes.org> <20180525114339.GA30990@sejong> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180525114339.GA30990@sejong> User-Agent: Mutt/1.9.2 (2017-12-15) Sender: linux-kselftest-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kselftest@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Fri, May 25, 2018 at 08:43:39PM +0900, Namhyung Kim wrote: > Hi Joel, > > On Wed, May 23, 2018 at 06:21:55PM -0700, Joel Fernandes wrote: > > From: "Joel Fernandes (Google)" > > > > This patch detaches the preemptirq tracepoints from the tracers and > > keeps it separate. > > > > Advantages: > > * Lockdep and irqsoff event can now run in parallel since they no longer > > have their own calls. > > > > * This unifies the usecase of adding hooks to an irqsoff and irqson > > event, and a preemptoff and preempton event. > > 3 users of the events exist: > > - Lockdep > > - irqsoff and preemptoff tracers > > - irqs and preempt trace events > > > > The unification cleans up several ifdefs and makes the code in preempt > > tracer and irqsoff tracers simpler. It gets rid of all the horrific > > ifdeferry around PROVE_LOCKING and makes configuration of the different > > users of the tracepoints more easy and understandable. It also gets rid > > of the time_* function calls from the lockdep hooks used to call into > > the preemptirq tracer which is not needed anymore. The negative delta in > > lines of code in this patch is quite large too. > > > [SNIP] > > > > #ifdef CONFIG_IRQSOFF_TRACER > > +/* > > + * We are only interested in hardirq on/off events: > > + */ > > +static void tracer_hardirqs_on(void *none, unsigned long a0, unsigned long a1) > > +{ > > + if (!preempt_trace() && irq_trace()) > > + stop_critical_timing(a0, a1); > > +} > > + > > +static void tracer_hardirqs_off(void *none, unsigned long a0, unsigned long a1) > > +{ > > + if (!preempt_trace() && irq_trace()) > > + start_critical_timing(a0, a1); > > +} > > + > > static int irqsoff_tracer_init(struct trace_array *tr) > > { > > trace_type = TRACER_IRQS_OFF; > > > > + register_trace_irq_disable(tracer_hardirqs_off, NULL); > > + register_trace_irq_enable(tracer_hardirqs_on, NULL); > > return __irqsoff_tracer_init(tr); > > } > > > > static void irqsoff_tracer_reset(struct trace_array *tr) > > { > > + unregister_trace_irq_disable(tracer_hardirqs_off, NULL); > > + unregister_trace_irq_enable(tracer_hardirqs_on, NULL); > > __irqsoff_tracer_reset(tr); > > } > > > > @@ -692,19 +650,37 @@ static struct tracer irqsoff_tracer __read_mostly = > > }; > > # define register_irqsoff(trace) register_tracer(&trace) > > #else > > +static inline void tracer_hardirqs_on(unsigned long a0, unsigned long a1) { } > > +static inline void tracer_hardirqs_off(unsigned long a0, unsigned long a1) { } > > Just a nitpick. These lines seem unnecessary since they're used > only when CONFIG_IRQSOFF_TRACER is enabled AFAICS. > > > > # define register_irqsoff(trace) do { } while (0) > > -#endif > > +#endif /* CONFIG_IRQSOFF_TRACER */ > > > > #ifdef CONFIG_PREEMPT_TRACER > > +static void tracer_preempt_on(void *none, unsigned long a0, unsigned long a1) > > +{ > > + if (preempt_trace() && !irq_trace()) > > + stop_critical_timing(a0, a1); > > +} > > + > > +static void tracer_preempt_off(void *none, unsigned long a0, unsigned long a1) > > +{ > > + if (preempt_trace() && !irq_trace()) > > + start_critical_timing(a0, a1); > > +} > > + > > static int preemptoff_tracer_init(struct trace_array *tr) > > { > > trace_type = TRACER_PREEMPT_OFF; > > > > + register_trace_preempt_disable(tracer_preempt_off, NULL); > > + register_trace_preempt_enable(tracer_preempt_on, NULL); > > return __irqsoff_tracer_init(tr); > > } > > > > static void preemptoff_tracer_reset(struct trace_array *tr) > > { > > + unregister_trace_preempt_disable(tracer_preempt_off, NULL); > > + unregister_trace_preempt_enable(tracer_preempt_on, NULL); > > __irqsoff_tracer_reset(tr); > > } > > > > @@ -729,21 +705,32 @@ static struct tracer preemptoff_tracer __read_mostly = > > }; > > # define register_preemptoff(trace) register_tracer(&trace) > > #else > > +static inline void tracer_preempt_on(unsigned long a0, unsigned long a1) { } > > +static inline void tracer_preempt_off(unsigned long a0, unsigned long a1) { } > > Ditto (for CONFIG_PREEMPT_TRACER). > > Thanks, > Namhyung Yes you're right, saves quite a few lines actually! I also inlined the register_tracer macros, It seems much cleaner. I will fold the below diff but let me know if there's anything else. Thanks Namhyung! - Joel ---8<----------------------- --- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c index d2d8284088f0..d0bcb51d1a2a 100644 --- a/kernel/trace/trace_irqsoff.c +++ b/kernel/trace/trace_irqsoff.c @@ -648,11 +648,6 @@ static struct tracer irqsoff_tracer __read_mostly = .allow_instances = true, .use_max_tr = true, }; -# define register_irqsoff(trace) register_tracer(&trace) -#else -static inline void tracer_hardirqs_on(unsigned long a0, unsigned long a1) { } -static inline void tracer_hardirqs_off(unsigned long a0, unsigned long a1) { } -# define register_irqsoff(trace) do { } while (0) #endif /* CONFIG_IRQSOFF_TRACER */ #ifdef CONFIG_PREEMPT_TRACER @@ -703,11 +698,6 @@ static struct tracer preemptoff_tracer __read_mostly = .allow_instances = true, .use_max_tr = true, }; -# define register_preemptoff(trace) register_tracer(&trace) -#else -static inline void tracer_preempt_on(unsigned long a0, unsigned long a1) { } -static inline void tracer_preempt_off(unsigned long a0, unsigned long a1) { } -# define register_preemptoff(trace) do { } while (0) #endif /* CONFIG_PREEMPT_TRACER */ #if defined(CONFIG_IRQSOFF_TRACER) && defined(CONFIG_PREEMPT_TRACER) @@ -753,18 +743,19 @@ static struct tracer preemptirqsoff_tracer __read_mostly = .allow_instances = true, .use_max_tr = true, }; - -# define register_preemptirqsoff(trace) register_tracer(&trace) -#else -# define register_preemptirqsoff(trace) do { } while (0) #endif __init static int init_irqsoff_tracer(void) { - register_irqsoff(irqsoff_tracer); - register_preemptoff(preemptoff_tracer); - register_preemptirqsoff(preemptirqsoff_tracer); - +#ifdef CONFIG_IRQSOFF_TRACER + register_tracer(&irqsoff_tracer); +#endif +#ifdef CONFIG_PREEMPT_TRACER + register_tracer(&preemptoff_tracer); +#endif +#if defined(CONFIG_IRQSOFF_TRACER) && defined(CONFIG_PREEMPT_TRACER) + register_tracer(&preemptirqsoff_tracer); +#endif return 0; } core_initcall(init_irqsoff_tracer);