From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A822747ACFE for ; Thu, 15 Oct 2026 07:26:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.44 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792049189; cv=none; b=e8ZIoIOx33UthhSZb/0bhtn/TSLT513h21V7h2oew+I/mqVPjBoe+SHicO4hrh2DKWcdDz+Flwmxdl0dqm6nxzAWT4N0wZc4yn0oHzVf73BcnewJ/HQ44n4pNsx59IUp1sFthZyOgn4mhAD8s6rwZNv8OAKwJ1ZIS1jabOR6w8o= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1792049189; c=relaxed/simple; bh=xwtBzirXEOCLJqRYZFFzZjl9ExUUK4CKgV7ms93Fd3w=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=BaNUye4GnZmM4R+lzDYfX9erHYMUf/y+yLckS4Lkd2VJkr03Vi0G4k6O5Vcw/HqZ4iRtmg/Dsm+x+YrfXbTZgjX+/HTuNWegpz1VSWiq5YY0LMhB1QdiCZtu9QOKPo5jOQ0zZhjC3C3nt7+6+Yvb/nYfHjTo0nABgSg3WMI9uEs= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=CmOgOEKt; arc=none smtp.client-ip=209.85.128.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="CmOgOEKt" Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-4a16af2a232so38446025e9.1 for ; Thu, 15 Oct 2026 00:26:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1792049178; x=1792653978; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to:content-type; bh=4TcvTv0+MA3VdoeuP+foXkM6LedLqAHNwCZW23zraGo=; b=CmOgOEKtLDPx5dFMZzstlEpO5tqqbN0GmUvZEOP+bNEI5ebj3yT6+MiiMGqHSBV6Jx MJrwDFv9411A7gxzZozTN7uA07+2fys4UdRO0KOs+WFaycBoOc+CMpoP9gBZDLU6aCXc 7pzeIPwG1hniWVl2u9yOlP7tAl8FAJfz59Qp5iPewo1p23kIqNC6ou5V55SyaAVEvn+h m7KJeJLS9+m3FhUS8bZkgzkE1s6ckq4/yI6lmikb8exXJoI3nNT8E2t6GmDIDX/l9FtL FzikmILsmC/r+6O7Jl+udQTLlwnjxB7mS3DTjnlc94g8ieSGDawgDCeCtgxzowLSLO+7 pv/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20260707; t=1792049178; x=1792653978; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to:content-type; bh=4TcvTv0+MA3VdoeuP+foXkM6LedLqAHNwCZW23zraGo=; b=zLWbeiT2LuMM1Afg+lw8Nxjxmkxghtvr2f+qo74Yyi88sn0RedhBFqt4gcKwN9jPiO 6s46MQ9U9VMdpggnQkbwT6vBT3b2LCZm49cicwVRFKNFPXcQybDKjPoF14iPBYTlUh5/ I85CtdYYqaRKOIGEKsngWBj0KZBdnA+20kF3n19UNfTp1hRynvNYshgJ0P2fjIaFakYa mYaPHAg02rBkxKLWRQKLS06KTMscD8mu/tBlsmdegBJm+f1C29pv1fk9kitMzb4Zwo9F N1mscpGkuCo3GkOIb7u/BDaZbmgbuUmVFZszEe+l3Medeex522gA3H+GJnL3y5r8kLt8 Xbbw== X-Forwarded-Encrypted: i=1; AKwUvBx8PT23h1saW+Yz23pCE4K25ZKB85UFraZlBJxSaJPgFq4bx6wc50lbXjrlbJ3BQDUFIyi11YvmwMT3D64=@vger.kernel.org X-Gm-Message-State: AFq9FYI+oYu9PTAKGInIduKfc30DykL71RiL97t17euLi1KlsUvYTBie /j/ZQw9ieUbLxY0KLYvbKnOdJwNj35Cff2WLS5AhXbvRDiYY3HW+rbOPPdAlyNnI X-Gm-Gg: AYBFou3bvOP48MJUDK3rrlwz9C5HJ0vxucDONPVHrfHLf73woOOPITRuHatAu/5Jtjf p7WiXArIAb9QbIYU11Xf5xtw/2QRfCEqQefMhmeK2v6owWLrJn6E9vaacdjEOXL+EnawITGjgQx zjpsStai3/Y6Y+E7rmOIn6Xhe6yj0orRTQGOJ9AX74Ntzg/dcNetIjQ4ebWIFWKEaoqYo3JDr4V rWUkiwTZC9A8AbX+CbpacrW/fiiGKCaSuwFU/vWLZICRTjnOHX4suAeM0hr6oaPl1FMTEBexwRi YC4aiKHdu7ggfEZJn1eL6TbFeZk8pLT0Z3Png0WMnW8XvE6gpSAhQ1USKieD/IuU1AoIWVedt4L 1hBlis9MfYSZNRu5BKpQeAF5VYsNYU0/G4NssK4jate1+hAlzSLnJIUkDdUIwYVpUNMhCQhnO0V nxMsZGHgehzCNnzH1oxPQK8SgJj4AC/tDvcQd5hTUHwWue+8+oXKQjvT20ftjVGuEIq08lmm9Im QXHcoVhRMT+NFt46zYWBgXHCowfV0nPmzXn99sZJJ57Sw== X-Received: by 2002:a05:600c:609b:b0:49f:ce78:356d with SMTP id 5b1f17b1804b1-4a18e4c9944mr358446205e9.30.1792049177294; Thu, 15 Oct 2026 00:26:17 -0700 (PDT) Received: from andreayoga.wind3.hub ([31.189.109.133]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4a1bd97ef4dsm55898465e9.4.2026.10.15.00.26.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Oct 2026 00:26:16 -0700 (PDT) From: Andrea Parri To: Steven Rostedt Cc: Andrea Parri , Masami Hiramatsu , Mathieu Desnoyers , linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org Subject: Re: [RFC PATCH] ftrace: Fix lost recursion records in ftrace_record_recursion() Date: Thu, 15 Oct 2026 09:26:11 +0200 Message-ID: <20261015072612.401602-1-parri.andrea@gmail.com> X-Mailer: git-send-email 2.53.0 In-Reply-To: <20261014172821.2b1fde91@gandalf.local.home> References: <20260917090217.6738-1-parri.andrea@gmail.com> <20261014172821.2b1fde91@gandalf.local.home> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit On Wed, Oct 14, 2026 at 05:28:21PM -0400, Steven Rostedt wrote: > > Can't claim to fully understand the logic behind ftrace_record_recursion(). > > Notably, its smp_mb__after_atomic() calls and the lack of barrier comments > > both look suspicious to my LKMM-trained eyes. ;) Hence the RFC. > > Well, the nr_records is updated via a cmpxchg() which itself is a full > memory barrier. The smp_mb__after_atomic() calls are to make sure the value > read is in sync with other CPUs before doing the compares. It also syncs > with the clearing function that is disabling and the writes, clears it, and > enables it (the -1 updates). > > Otherwise, IIUC, the atomic_read() itself doesn't guarantee that what you > read is what is in memory without explicitly using the smp_mb__after_atomic(). Thanks for the explanation. I don't think smp_mb__after_atomic() gives us that, though. Per Documentation/atomic_t.txt, the barriers smp_mb__{before,after}_atomic() "only apply to the RMW atomic ops"; atomic_read() and atomic_set() are not RMW ops, so after them the barrier is not guaranteed to order anything. Most architectures implement it as smp_mb(), which happens to cover these uses, but on x86 it is not even a compiler barrier, so the compiler is free to move the memset() before atomic_set(-1); LoongArch still defines it as barrier(), though a fix for that is pending [1]. And no barrier makes a load return a more recent value; barriers only order accesses against each other. What I think the code wants is: on the recording side, the load from nr_records ordered before the loads from recursed_functions[], e.g. atomic_read_acquire(); on the clearing side, the store of -1 ordered before the memset(), for which smp_wmb() is enough. (The memset() itself still races with the recorders. Marking its stores would make that a well-defined race, but clearing would stay best-effort, as the existing comment says, unless the clear waits for in-flight recorders, e.g. with a grace period.) This is independent of the cached_function bug, so I'll leave it out of v1 and send it as a separate patch, with comments, if that's OK with you. > > @@ -67,24 +67,27 @@ void ftrace_record_recursion(unsigned long ip, unsigned long parent_ip) [...] > > old = cmpxchg(&recursed_functions[index].ip, 0, ip); > > if (old != 0) { > > /* Did something else already added this for us? */ > > - if (old == ip) > > + if (old == ip) { > > + cached_function = ip; > > return; > > + } > > I think we do not need the above. If old == ip, then whatever set it before > should have updated the cached_function. Makes sense, I'll drop it in v1. (The cache only holds one entry, so another ip could have replaced it in the meantime, but that just means a later call takes the slow path.) Thanks, Andrea [1] https://lore.kernel.org/r/20260908073906.31781-1-yangtiezhu@loongson.cn