From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f50.google.com (mail-wm1-f50.google.com [209.85.128.50]) (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 A95B937F303 for ; Wed, 26 Aug 2026 08:40:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.50 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787733631; cv=none; b=jOUrPqIZY3Pjo4xBbnqxrA+8ri3ZbV5D3WyVaPd6mSHmXt1eiA348BhW2fpxCM7nwsMGgmeny1MRcPmhJxzLBHpFm1RJhrV8e3E7XZn3WfjTZ1IxgnViUYRMG//7vRFjCb4dSOlhWLb7C+njWhEU5rs/8z6Bs4HqLXvheN4VUCo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787733631; c=relaxed/simple; bh=cUJtRsZoK1io9Vg9mrpUB95/eLRug5gLwoJM1325w38=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=UeX7RUK0x174duE3nEZJHyKN44P+0UZ1P/hmzgygTUSwF/ip1Op32xRFDoPI5G7D4SV+FLrxbGEA9BqC76Cl7wFl/4ADm32Sfot4ABK6uS9li+gnaN5Y3hiJw08nLrGENmtnec6Zw9PwNNB8F3Gc+3Pczut1HwqEZI+1amIyzaM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=i/PWDk4/; arc=none smtp.client-ip=209.85.128.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="i/PWDk4/" Received: by mail-wm1-f50.google.com with SMTP id 5b1f17b1804b1-499b02fc590so2298695e9.2 for ; Wed, 26 Aug 2026 01:40:29 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1787733628; x=1788338428; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=/K+neFx7uoZLZLhEPqSvYz4Rq8Bad8pRnsZy2NkitOQ=; b=i/PWDk4/OzrhC18aKtC3C23Yftsxmjc6JrRCJ6HPP+Mr1Ja6Qlkvxck9ckElbi7N3I 1rvrE2p5Req7yLe147GiDADIEpRlK2ESdmKKe8Twd4Qmmit/OrTg5JSlhE75o9zjNDO+ /A0HuSCrrOr2PidDtOCR10BDUkzLs4PCri2N+BoLB11Kxcbh8H5BxSzjujCIoosOlmU+ xJaOco4RwGDJ6gYUud06fnVcJOS8cubHS5+GNPfO/Owep2ly8BBeRzyMI4V9kvHMVDpw 2XKrzge7XgfCK5GCrzxti9tNVqcy/+0hHfasHO5nh2jvu8uUnTBF44IxbElUr+omsNJL nVTw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787733628; x=1788338428; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=/K+neFx7uoZLZLhEPqSvYz4Rq8Bad8pRnsZy2NkitOQ=; b=ZX5M4F5LEyp9WkvTDhOlvTXmdeJ9WXLqWJ5uQ65Nmm8ox80UMKmquylRFQux/5NgCr d9eeXSf3MFdtlwo6bmK6bhT0Nb5sGMw6g0CLIs9YUpOM3/i8MGge2E+nAxdKBLHw9s8N 2prHiuLFcx2Y/NFZaGTfcxETiVQhjKSqNF/IhjrRoQMevvCW7Q8CuDcKe0Vkt+3HZbix QWqJpeccmwtLAoI1oS2T2HMQh0m+IkMDjq1OfEUZyd83ao8+lrgFoekfi3I62eDXlPVe 3iq8Uu8cgjoZxFiMgZDMxjpeVjJ33YgLzWjTW5iJkWX3svkjBdKwVbea1KF/gbvsHYLP pOrQ== X-Forwarded-Encrypted: i=1; AHgh+RpSv4RI/hoSfb2kQpC62DOvjPsht8PwptwanY2RIyAMDvtpswRB3Ak/hKJU5SNZrYV8cndMwIysxIeDlc8=@vger.kernel.org X-Gm-Message-State: AFuF++m30bfsVtiYr0nPJPdkatBo5M2TiRLYg3lua4BEKD5IDTx2F09b psCvBqFSKF/yt4yD716PaPooNElC6wo9FZhPyYZGS/F244ArUMnc+toKVi5Tm25DGYsBaHOSU89 C4KuUKNg= X-Gm-Gg: AR+sD111+n5a+g5LMbbnDlFDdbQCuJGaPH19SDtHCvabNoUEP1h5QIEwmRSQ22sYu+l hgzPx50FKYzpEhBUVmHjqmbm22ecktkh/T/r4o+D5tZmDCnLMOtXCB5JYM75GFDIv6/0f8tMjNn k0x0CKnB45zQZ82icypphiCANrJUoKRCSHMlFMNKzhLZy/xMtD9bWQ7y+FYopYtuRb4mnbo7kOY 2dqhh3v7PYVDvbxexSTTBZxKf5rhk6oWDGDZMwpdS1YFElDH2g249Nn9N4BGBg25nZ5OBCsDwaD 70Li19UpZQ2UyqEqnFXYEhhpc4GWTlF07pRYUKWuMsNgWDcrW1Va2uabtgvQlLkzqsuaUapvQSF eyZrBk7fhUXZtOn4HOLBxYCgK4nH7Jjib2+dXbAO0h0ig0hzWhbrz+hpBq5sSyDRcfuAfdO4yT9 RwRrxCH6rwLPOGZd35lzL6gs84x3Mc6u4did543QAP/9kdbRAJ98W7G/bGyAgcXw== X-Received: by 2002:a05:600c:8518:b0:499:a5c8:c6f3 with SMTP id 5b1f17b1804b1-499dc6e998cmr45290165e9.3.1787733627757; Wed, 26 Aug 2026 01:40:27 -0700 (PDT) Received: from [192.168.1.3] ([37.18.141.193]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-482e27ab574sm2001609f8f.14.2026.08.26.01.40.26 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 26 Aug 2026 01:40:27 -0700 (PDT) Message-ID: Date: Wed, 26 Aug 2026 09:40:26 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 4/8] coresight: tmc-etr: Prevent per-thread events from sharing a sink To: Leo Yan Cc: Suzuki K Poulose , Mike Leach , Suyash Mahar , Yeoreum Yun , Greg Kroah-Hartman , Qi Liu , Junhao He , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jonathan Cameron References: <20260728-james-cs-multiple-per-threads-v3-0-6aee7579f1dc@linaro.org> <20260728-james-cs-multiple-per-threads-v3-4-6aee7579f1dc@linaro.org> <20260813160504.GC8904@e132581.arm.com> <20260819084515.GF8904@e132581.arm.com> <20260825173220.GG8904@e132581.arm.com> Content-Language: en-US From: James Clark In-Reply-To: <20260825173220.GG8904@e132581.arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 25/08/2026 18:32, Leo Yan wrote: > On Thu, Aug 20, 2026 at 12:09:25PM +0100, James Clark wrote: > > [...] > >>> I am just wandering if we can improve the sink driver to only allocate >>> a single bounce buffer that is independent of any threads (and any >>> associated events). >>> >>> | T1 | >>> CPU0 ------------------------------ >>> | T2 | >>> CPU1 ------------------------------ >>> `> T2 stops and can sync trace >>> from the shared bounce buffer >>> to AUX_BUF(T2). >>> >>> AUX_BUF(T1) | | >>> AUX_BUF(T2) | | >>> >>> ETR_BUF | Bounce buf | -> Used by H/W trace >>> >>> This might also simplify the CPU-wide case. Each CPU would still have >>> its own AUX buffer, but the ETR driver would maintain only one bounce >>> buffer for the shared sink. A reference count could track how many >>> events are using the sink, with the final event responsible for What size and mode (CATU, FLAT, etc) would the one bounce buffer be? Isn't the problem that a bounce buffer per-session solves that the user can pick a different size and mode for each session? >> >> Isn't this how it's already working? get_perf_etr_buf_cpu_wide() allocates a >> single shared buffer with a refcount. I didn't change this, I only changed >> the rules about what is considered shared or not so that it matches the >> semantics of the perf events that back the tracing session. > > I think this is slightly different from my point. > > The CPU-wide path already uses a shared buffer with a reference count > to support multiple events, while the per-thread path does not. > > For the longe term, I would prefer to unify the sink buffer management. > Ideally, ETR/ETF/ETB should manage the sink buffer in the same way > regardless of whether the users come from CPU-wide or per-thread modes. > This would keep perf event semantics out of the low-level sink drivers > as much as possible. However, this would be a larger change and we could > defer in the future. > I suppose I'm a bit confused about how it relates to this fix. All the buffer management stuff is transparent to the user, it doesn't really matter how the driver does it. This fix fixes a bug relating to a user visible behavior. If there is a way to simplify the buffer management and preserve all of the use cases we can do it, but I don't see why it needs to be done now rather than later. > Now I treat the multiple events in per-thread mode as an implementation > limitation. For the immediate fix, we just reject this case instead. > >>> We use a central place etm_event_build_path() to record and compare >>> event's owner and target process, then we don't need to spread the >>> check into sink drivers. We only care about if owner and target must >>> be consistent. > > I experimented with moving the check to a common place during buffer > allocation: https://termbin.com/dib3r > > It needs locking to keep the check in atomicity, but seems doable. We > don't need to spread event checks across the different sink drivers. > I agree it would be possible to do, my only argument is that it doesn't support all the uses cases that we currently support and that are likely in use. >> But we don't know where the target will run when the event is created. >> That's why the check is delayed until etm_event_start() and the process has >> been scheduled. Where it runs needs to be taken into account to calculate if >> this sink can be shared. > > Adding the check in etm_event_start() makes the result depend on task > scheduling. I didn't add it, it's already there for owner PID so it already depends on scheduling. And not checking target TID leads to a WARN on etm_event_start() as well. So nothing is being added, just the existing check is being made more restrictive and the WARN is fixed. > > I understand some cases you mentioned may benefit from this, but it also > makes the behaviour less deterministic. I would prefer to reject > unsupported cases explicitly when opening the events. > You didn't explain how you would account for taking away users' currently working use cases. Two users using taskset to run on two different cores does behave deterministically. But you are taking that away from them by preventing any event from being opened that _may_ share a sink in the future, even if it never does. Taking away concurrent per-thread sessions seems like a huge functionality loss. We can agree to do it and change that behavior, but that's a completely different change than fixing this bug with the existing behavior, and I'm not sure what the justification for it would be. "Determinism" seems like a weak argument when the fix is to make the driver much less flexible and useful. I don't think we would want that change to be a "fixes:" commit either. There is some chance it would have to be rolled back if someone complains, and then we'd get the WARN back again. > Thanks, > Leo