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=-9.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,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 6CFB4C433B4 for ; Tue, 27 Apr 2021 03:47:41 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 CE676611BE for ; Tue, 27 Apr 2021 03:47:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CE676611BE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=b1EEq3Xh8uaQbXvtDD1vhWcKsBTGWkrTcVA6c8bahjg=; b=bdn+sTAfeyWJr03vkFWaTwV6Q Awc8Xs4wNcMxTLd8lYsiDgVI27cPMrGT0j79jTCIXMC7saE1q3QNuTviR+DmAgLb3vzlgnRVD3ZMO AWbxd9w2OcHAPaj0Wt/YgzQ/63Nk6hVDxJfJfvUfDzfs7hicxYgNYijWhwO4W0fS1RdyVU6OHyAWb lmcXJUw3UgTN7xTA9sxrPsjuqx5lcKGe5ikxTngza0s8WthJRyvt/NNWIg8jlKLQCgMpNaK0wB992 VnRrkR0gKl6vgcuj5lhkvw/tySFLcLZp9ZHQRGh5p4Lm2FU0k16KFqlWAIT/MitB4sTWbsR8Jx8RX 1ToL3su4g==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lbEfW-000bFB-RR; Tue, 27 Apr 2021 03:45:51 +0000 Received: from bombadil.infradead.org ([2607:7c80:54:e::133]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lbEfQ-000bDZ-HO for linux-arm-kernel@desiato.infradead.org; Tue, 27 Apr 2021 03:45:44 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=GZF0DajT2j4S1nzv7gyliz1cjwb3ldNPaCeRF2Yozco=; b=ckGH83kHnN9SpEp2Y3srSTsmhL zCpoffH/+yeCDW6ZQ5D3WkEq3XmjcMEP+BowH6hVHlVRvOnc+jIL/tbHNBwk+l8cLIdDsFILhO8jo Cs+KT8/IutOE2fabCK/5r1oQhZoas088P/oNMVRdTTknHgOhGCqVgH29ff/Z50dcdwcxehcf2Go2N 4EDPknBY6eoIbHUSLcLKZ/Sf5vbLPSgFcZEEcxaGTxx+L73lG3c+kEi7jLrJ7VK8rAETo1g5xak3f z0rXjgp80QYWzKzQliMmskkxcPeXkPhXymfyESKK0fHaHX5t2JUqKpkxTHlikV7N52XnbCEO9w+0R cMPOIODQ==; Received: from mail-pj1-x1029.google.com ([2607:f8b0:4864:20::1029]) by bombadil.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lbEfN-00GPTx-Oi for linux-arm-kernel@lists.infradead.org; Tue, 27 Apr 2021 03:45:43 +0000 Received: by mail-pj1-x1029.google.com with SMTP id y22-20020a17090a8b16b0290150ae1a6d2bso6511026pjn.0 for ; Mon, 26 Apr 2021 20:45:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=GZF0DajT2j4S1nzv7gyliz1cjwb3ldNPaCeRF2Yozco=; b=WWaflSeilWImP3hp6Td4FGj3LjpAiVnGLlHbmrxDKONmsBEGP1of9PyY1j3Q6gm4pR KRWwl73upfJtOeDc2EhZmM1EmJMt53K8OWWmY4cmQEvovskpYOWqoUH6yGNm7cRtBBZY CJ1G4eRYv3e/SFiklI23H4RzIuPrtFdiHbDaLs/KeyOtrTDMcXqstDTCmU5ZJGzxIakI X1IgnbCE2RjlrrMfsOdITTPajh5+2PZPhH4cXM0hGZrEG+SlSuE2QOOahlHWknokxJb+ e61XdnjeD6HHaj8biU3CXIn5SnSAT/yEYmRWbeoFMf9h/Hnl/QQx2SAi6qWWE0eXu+A8 Yqgg== 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; bh=GZF0DajT2j4S1nzv7gyliz1cjwb3ldNPaCeRF2Yozco=; b=ae2czA3IvmPFKlKao8kQK7yUUypdV7qlZ+NKedWoX5SlrUBDNB74kThnnCScxI47FS Iyweu254j/HwAasALKcYofjW9wJTqeBzeYHd+xxncU1t0BMXQ+uzV7TJAgqFmLC7fAn0 sfq90n6wKSvc1nuXpqrQH8azQQAMQD5NPFVSGcdIQYj2HAPsC9Lk++V9Q00F2hh1NKOL wIhv0xlPBcnS+YIOlqHewLNHqBexHjbF3BU4AbpXjJufLfN8M7pHUYqQvSxTyymsVJu5 ZURJ/SHnTQf8d1TCG0VVtqiNbwn/hzMRRtd5s+/HcyA3mxdGdKEe7kdTknhrpfaJj3WB D0yg== X-Gm-Message-State: AOAM530+UygGa9A1ojlZzfT7XyMBgRCmn9a1QTaYcSEMoMaEaBo4oZW5 D9EDiDoO+GK7grl45TPo7MZX/g== X-Google-Smtp-Source: ABdhPJyMe/K1iO7isZzTig1c0tLE8rxy+GfkZ2XwhAXYcXGq1R6hKfbHThIe4daFag9Lw2vsP1RApA== X-Received: by 2002:a17:90b:17c9:: with SMTP id me9mr2559322pjb.233.1619495138780; Mon, 26 Apr 2021 20:45:38 -0700 (PDT) Received: from leoy-ThinkPad-X240s ([116.206.101.232]) by smtp.gmail.com with ESMTPSA id b13sm962370pjl.38.2021.04.26.20.45.35 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 26 Apr 2021 20:45:38 -0700 (PDT) Date: Tue, 27 Apr 2021 11:45:31 +0800 From: Leo Yan To: Suzuki K Poulose Cc: Daniel Kiss , mathieu.poirier@linaro.org, mike.leach@linaro.org, coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, denik@google.com, Branislav Rankov Subject: Re: [PATCH 1/4] coresight: tmc-etr: Advance buffer pointer in sync buffer. Message-ID: <20210427034531.GA328795@leoy-ThinkPad-X240s> References: <20210421120413.3110775-1-daniel.kiss@arm.com> <20210421120413.3110775-2-daniel.kiss@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210426_204541_862882_1865A4DD X-CRM114-Status: GOOD ( 27.55 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Mon, Apr 26, 2021 at 11:40:44AM +0100, Suzuki Kuruppassery Poulose wrote: [...] > > @@ -1442,7 +1442,7 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf, > > { > > long bytes; > > long pg_idx, pg_offset; > > - unsigned long head = etr_perf->head; > > + unsigned long head; > > char **dst_pages, *src_buf; > > struct etr_buf *etr_buf = etr_perf->etr_buf; > > @@ -1465,7 +1465,7 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf, > > bytes = tmc_etr_buf_get_data(etr_buf, src_offset, to_copy, > > &src_buf); > > if (WARN_ON_ONCE(bytes <= 0)) > > - break; > > + return; > > bytes = min(bytes, (long)(PAGE_SIZE - pg_offset)); > > memcpy(dst_pages[pg_idx] + pg_offset, src_buf, bytes); > > @@ -1483,6 +1483,7 @@ static void tmc_etr_sync_perf_buffer(struct etr_perf_buffer *etr_perf, > > /* Move source pointers */ > > src_offset += bytes; > > } > > + etr_perf->head = (pg_idx << PAGE_SHIFT) + pg_offset; > > > Looking at this patch, I feel the driver is doing a couple wrong things > already. > > 1) We initialise etr_perf->head every time the ETR enable is called, > irrespective of whether we actually try to enable the Hardware. e.g, > > etm_0 on -> .. -> enable_etr : > etr_perf->head = > enable_hw() > > emt_1 on -> ... -> enable_etr: > etr_perf->head = > already_enabled, skip enable_hw() > > etm_2 on -> ... -> enable_etr: > etr_perf->head = > already_enable, skip enable_hw()... > > > This doesn't look correct as we don't know which handle is going to get the > data. This looks pointless. I'd like to convert mapping into below diagram (for system wide trace): CPU0: AUX RB (perf_output_handle_0) -> etr_perf -> +---------+ CPU1: AUX RB (perf_output_handle_1) -> etr_perf -> | etr_buf | CPU2: AUX RB (perf_output_handle_2) -> etr_perf -> | | CPU3: AUX RB (perf_output_handle_3) -> etr_perf -> +---------+ Simply to say, there have two layers for controlling ring buffer, one layer is for perf AUX ring buffer, it mainly uses the structure perf_output_handle to manage the ring buffer. And in the ETR driver, it uses structure etr_perf to manage the header pointer for copying data into ETR buffer (tagged as "etr_buf"). ETR buffer is the single one, but the structures "perf_output_handle" and "etr_perf" are per CPU. We have multiple copies for the headers and tails to manage a single buffer, but the problem is these multiple copies have not been synced with each other. > 2) Even more problematic is where we copy the AUX buffer content to. > As mentioned above, we don't know which handle is going to be the last > one to consume and we have a "etr_perf->head" that came from one of the > handles and the "pages" that came from the first handle which created a > etr_perf buffer. In sync_perf_buffer() we copy the hardware buffers to > the "pages" (say of handle_0) with "etr_perf->head" (which could be from > any other handle, say handle_2) and then we could return the number of bytes > copied, which then is used to update the last handle (could be say > handle_3), where there is no actual data copied. > > To fix all of these issues, we must > 1) Stop using etr_perf->head, and instead use the handle->head where we are > called update_buffer on. > > 2) Keep track of the "pages" that belong to a given "handle" and then use > those pages to copy the data to the current handle we are called to update > the buffer on. The "pages" are only allocated once, even they are attached to multiple handles. I think the right way is to use the single structure "etr_perf" and single "perf_output_handle" to manage the "pages", IOW, if there have single buffer, then we just use one copy of header and tail to manage it. The difficult thing is how to use the single one "perf_output_handle" to manage the AUX ring buffer and notify to user space. I am wandering if we can only use CPU0's perf_output_handle to manage the AUX ring buffer, if any other CPUs read out the data, they always use CPU0's perf_output_handle. Thanks, Leo _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel