From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id ABB51C77B70 for ; Sat, 15 Apr 2023 15:46:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229995AbjDOPqk (ORCPT ); Sat, 15 Apr 2023 11:46:40 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34724 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229670AbjDOPqk (ORCPT ); Sat, 15 Apr 2023 11:46:40 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [IPv6:2604:1380:4641:c500::1]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E9ACA2D59; Sat, 15 Apr 2023 08:46:38 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 886C160AEE; Sat, 15 Apr 2023 15:46:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7182EC433D2; Sat, 15 Apr 2023 15:46:37 +0000 (UTC) Date: Sat, 15 Apr 2023 11:46:35 -0400 From: Steven Rostedt To: Yafang Shao Cc: Alexei Starovoitov , LKML , Linux trace kernel , Masami Hiramatsu , Mark Rutland , bpf Subject: Re: [PATCH] tracing: Add generic test_recursion_try_acquire() Message-ID: <20230415114635.15d9eda8@rorschach.local.home> In-Reply-To: References: <20230414221702.554c44fe@rorschach.local.home> X-Mailer: Claws Mail 3.17.8 (GTK+ 2.24.33; x86_64-pc-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-trace-kernel@vger.kernel.org On Sat, 15 Apr 2023 22:33:17 +0800 Yafang Shao wrote: > > I don't have a clear idea why TRACE_CTX_TRANSITION must be needed, but > it seems we have to do below code for the fentry test case, The issue is that there's still cases that we can trace when preempt_count hasn't been updated to the new context. That is, preempt_count is used to determine if we are in softirq, hardirq or NMI context. But there's some places that have: normal context: func_a() --> traced --> interrupt func_b() --> traced preempt_count_add(HARDIRQ_OFFSET) Now we drop the second trace because it is flagged as a recursion when in reality it is in a new context, but the preempt_count has not been updated yet. We are currently fixing these locations, but it's not there yet. And since there's tools that relies on not dropping these locations, the transition bit needs to be there until this situation is dealt with. Can you change the tests to allow a single recursion? -- Steve > > diff --git a/include/linux/trace_recursion.h b/include/linux/trace_recursion.h > index d48cd92d2364..f6b4601dd604 100644 > --- a/include/linux/trace_recursion.h > +++ b/include/linux/trace_recursion.h > @@ -163,21 +163,8 @@ static __always_inline int > trace_test_and_set_recursion(unsigned long ip, unsign > return -1; > > bit = trace_get_context_bit() + start; > - if (unlikely(val & (1 << bit))) { > - /* > - * If an interrupt occurs during a trace, and another trace > - * happens in that interrupt but before the preempt_count is > - * updated to reflect the new interrupt context, then this > - * will think a recursion occurred, and the event will be dropped. > - * Let a single instance happen via the TRANSITION_BIT to > - * not drop those events. > - */ > - bit = TRACE_CTX_TRANSITION + start; > - if (val & (1 << bit)) { > - do_ftrace_record_recursion(ip, pip); > - return -1; > - } > - } > + if (unlikely(val & (1 << bit))) > + return -1; > > val |= 1 << bit; > current->trace_recursion = val; > > > > The reason is that the bpf_map_delete_elem() in this fentry SEC()[2]