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 X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 736BDC43381 for ; Mon, 1 Apr 2019 13:02:11 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 44F682084B for ; Mon, 1 Apr 2019 13:02:11 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="cOkCjO9M" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 44F682084B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date:Message-ID:From: References:To:Subject:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=85ArwtpwkTiBuxfXdK5qx9jWg5hbEUCFK5ivUvNUyLA=; b=cOkCjO9MlFNVtDIn/z3DwbNKW MgQ24A9JiDMW0eqEQBAtyxdkdBnuxAtLk2sBJF13u+aMWwHjHTKCvEd39fFB7Zj8Mg9PUgb3mNfPY tvjtA8ejVtLB0jkG2Zm2Yb9+USjxzxWBySilpVDWW96vTubqwp0ucK8fP3CbTcC6YKyTrvSJmXXfZ UWB9hhr8BzSeQ4cEW/iYoLcbYB7c1b9etl+YWB1XuubyH+XDCE3CpNsrgdHRzzAaGLV8ymMu0uHNb LJ/Sjaa5YfM0xFH8Cqg65gyRM+ORwXqBHpzeoejW3L6yGovhdjJhnGmzDku+ZyGtMSTX4k6WIQuNj 1gvAZdYFw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hAwZi-0006IO-24; Mon, 01 Apr 2019 13:02:06 +0000 Received: from usa-sjc-mx-foss1.foss.arm.com ([217.140.101.70] helo=foss.arm.com) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1hAwZe-0006HX-Fp for linux-arm-kernel@lists.infradead.org; Mon, 01 Apr 2019 13:02:03 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 0169780D; Mon, 1 Apr 2019 06:02:00 -0700 (PDT) Received: from [10.1.196.93] (en101.cambridge.arm.com [10.1.196.93]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1646A3F557; Mon, 1 Apr 2019 06:01:55 -0700 (PDT) Subject: Re: [PATCH v2 13/16] coresight: tmc-etr: Allow events to use the same ETR buffer To: mathieu.poirier@linaro.org References: <20190325215632.17013-1-mathieu.poirier@linaro.org> <20190325215632.17013-14-mathieu.poirier@linaro.org> <20190326175525.GA17902@xps15> From: Suzuki K Poulose Message-ID: <48ddc78c-0414-5233-bf8e-c1597b052298@arm.com> Date: Mon, 1 Apr 2019 14:01:52 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.5.1 MIME-Version: 1.0 In-Reply-To: Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190401_060202_538575_B5D461D0 X-CRM114-Status: GOOD ( 24.75 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: alexander.shishkin@linux.intel.com, coresight@lists.linaro.org, peterz@infradead.org, Mike.leach@arm.com, leo.yan@linaro.org, linux-arm-kernel@lists.infradead.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Mathieu, On 27/03/2019 17:01, Mathieu Poirier wrote: > On Wed, 27 Mar 2019 at 05:30, Suzuki K Poulose wrote: >> >> On 03/26/2019 05:55 PM, Mathieu Poirier wrote: >>> On Tue, Mar 26, 2019 at 04:18:35PM +0000, Suzuki K Poulose wrote: >>>> On 03/25/2019 09:56 PM, Mathieu Poirier wrote: >>>>> This patch uses the pid of the process being traced to aggregate traces >>>>> coming from different processors in the same sink, something that is >>>>> required when collecting traces in CPU-wide mode when the CoreSight HW >>>>> enacts a N:1 source/sink topology. >>>>> >>>>> Signed-off-by: Mathieu Poirier >>>>> --- >>>>> .../hwtracing/coresight/coresight-tmc-etr.c | 71 +++++++++++++++++-- >>>>> 1 file changed, 65 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/drivers/hwtracing/coresight/coresight-tmc-etr.c b/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>> index 7254fafdf1c2..cbabf88bd51d 100644 >>>>> --- a/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>> +++ b/drivers/hwtracing/coresight/coresight-tmc-etr.c >>>>> @@ -8,6 +8,7 @@ >>>>> #include >>>>> #include >>>>> #include >>>>> +#include >>>>> #include >>>>> #include >>>>> #include >>>>> @@ -41,6 +42,9 @@ struct etr_perf_buffer { >>>>> void **pages; >>>>> }; >>>>> +static DEFINE_IDR(session_idr); >>>>> +static DEFINE_MUTEX(session_idr_lock); >>>> >>>> Please correct me if I am wrong here. What we now do with this series is >>>> >>>> - One event per CPU and thus one ETR perf buf per CPU. >>>> - One *ETR buf* per PID, from the IDR hash list. >>>> >>>> However, if we have 1:1 situation, we will have : >>>> >>>> N (say 2 ETRs), but one *ETR buf* as they all share the same PID, implying >>>> multiple multiple ETRs will try to use the same etr buf, >>>> which could corrupt the trace data ? Instead, what we need in that >>>> situation is : >>>> >>>> One ETR buf perf PID+ETR device. i.e, etr_bufs must be per ETR. >>>> So should the IDR be specific to an ETR ? >>>> >>>> Or do we not support a session with multiple ETRs involved (1:1) ? >>> >>> At this time 1:1 topologies are not supported and a fair amount of work will go >>> in doing so. When I started working on this serie my thought was that because >>> of said amount of work there is no point thinking about 1:1, and that we can >>> deal with it when we get there. >>> >>> But taking a step back and having dealt with the harder (concurrency) problems >>> inherent to CPU-wide scenarios, it would be trivial to make the code 1:1 ready >>> and it will be one less thing to worry about down the road. >>> >>> That being said and answering your question above, I think we need and IDR per >>> ETR (in the drvdata) to avoid contention issues on systems with several ETRs. >>> >>> Thanks for bringing this back to my attention. >> >> Cool. Thanks for explaining the rationale. So, when we do that, I think >> we may be able to have one "etr_perf_buffer" per ETR and thus we may be >> able to move the refcount back to the etr_perf_buffer, just like we >> moved the PID and index etr_perf_buffer instead of the etr_buf ? > > An etr_perf_buffer is associated with an event and holds the AUX ring > buffer that was created for that event. In CPU-wide N:1 mode multiple > events (one per CPU), each with its own AUX ring buffer, share a sink > and as such we can't have a single etr_perf_buffer per ETR. Ok. Thanks for clarifying this. So we have one AUX buffer per event, but they all could end up in the same ETR HW buffer and may be copied to only one of the AUX buffer, which happens to really disable the ETR. And thus we need to have a sufficiently large AUX buffer in place to allow we don't overflow all the time with trace from multiple events. Makes sense for the N:1 topology. Also the decoding phase must extract the trace to the corresponding event based on the TRACEDI of the packets. Thats the best we could do at the moment. If there was a way to have one single large AUX buffer for a set of events, which is easily accessible from a given event in the set, rather than "N" large buffers which could potentially impact memory consumption. It would be good to have this clearly documented somewhere in the implementation to keep us in track. Cheers Suzuki > > Thanks, > Mathieu > >> >> Cheers >> Suzuki >> _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel