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=-13.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER, INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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 41323C433DB for ; Sat, 20 Feb 2021 08:13:31 +0000 (UTC) Received: from merlin.infradead.org (merlin.infradead.org [205.233.59.134]) (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 EE48164EB4 for ; Sat, 20 Feb 2021 08:13:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org EE48164EB4 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=merlin.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References:Message-ID: Subject: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=+EIekCupwlzLZm1DfEzrDcsOCqUk/OCZHmjNe8W4pus=; b=fYSk3rIh8fD1RiTKg+LEd6sGi PnNC1UgsINEwbYGfMl9Cv6+boQjdYk1i2CCJbrRcA5sSnaenoVTOJ2hidwZEWGxmL26u8FoQCSQkk O+LPpZ+9OTCTAOkrgQdEJCjfFjLU+T+wulyuYyqvboF4RtKWnhCNyNABfHjCnoqxgEdZLBBqerpaF pAkbO/kaQXk1AlQm17/RNjZFiPFu9v22mqyA0gdqN+oNAXwhdQk6hJ1kWgGaEoyc9lWtAGH2Rm9Qs E641MFM8SLIeMoUkr/BIEn1iLPsv3kFyqQXnU7yBzwgOk5F26knvirrMM8v1UCMzS7pIIPQxc+L4V XawUEev2w==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1lDNMM-0002Vq-7L; Sat, 20 Feb 2021 08:11:26 +0000 Received: from mail-pg1-x52c.google.com ([2607:f8b0:4864:20::52c]) by merlin.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1lDNMJ-0002VK-IW for linux-arm-kernel@lists.infradead.org; Sat, 20 Feb 2021 08:11:25 +0000 Received: by mail-pg1-x52c.google.com with SMTP id z68so6864143pgz.0 for ; Sat, 20 Feb 2021 00:11:20 -0800 (PST) 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=V+9eRXBXr3a/Jjk1oqfCZbLnBKfF/Q5hWwSEoBYXKsI=; b=W1YC8dkBKHnTBnbSX8mBp/ciNjiZtm+Dw6hISPi85dC+wnfU6e6Sh+pIKSxyuzznPQ BiC864ubG6IUeo6w2TWSojojwi4XP9f/QmVMcUNY3EXk+v/yzwooaToRSKjss8jFTT7Q WYpj1uqT08VjdJ9EaS0PvSJTB42mHDHuWQS+zwEg6hNm+janpTgA5YgIegenh7zvyLyg WEEQIUhmxGKgcIVAB2G2fNZnCb7/YPB07+xlp4ojjmg0XWsKGevAbAVIRE82ncBub6Hh glaXoNwACmC3CXAPcOoHpOxdscZ9Z40MPOvVfPeH372MvES+X2TMQRnfDI4Tp9/YqC+F dCkg== 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=V+9eRXBXr3a/Jjk1oqfCZbLnBKfF/Q5hWwSEoBYXKsI=; b=tNQpcl/gJRR5261d5FsgQNT5DmlL8e3WlOPriBJBmfW8gD0HD+SbUJhRpJhiWRHkpL iGCN11RbQkhDkS+2KxuLrUqFqtxhaWwVq4myBfWpiruJQZAJKcj1KHyCtCUVX9T08dyk E1VYFTr7JHxCXWzCP4hOUgVHiAoXu8CdgZUh8srM2qcRHK2UB0iYIOG/EkOaVe57IuOl IRjzjuojxYUPKbVZgsqIcwrcADC2uNS6V+V4NhHpYvVwVKt1ut++zxM8h227VQVcneFG 26y6o9ZsK9s8lKn5ayAOCcRtbwACclVeLsMTCaqtIGp1FleoAtwPEIL3BhfzTrkKQqUx Ub4g== X-Gm-Message-State: AOAM532iPIzA6ZYWkYFpvXkE1btheGMnInmtRvpcV+JX2CJmTKQ39BNd OfgR266cZhsw+AlG+V3/uE/jIg== X-Google-Smtp-Source: ABdhPJydj1PELrX33LMT6/dBfXUKNCO9kgla7zUsILNl2j83CZTMGIm/PTA6YBveusVU2TYeRrwRXQ== X-Received: by 2002:a63:b66:: with SMTP id a38mr11615309pgl.116.1613808678557; Sat, 20 Feb 2021 00:11:18 -0800 (PST) Received: from leoy-ThinkPad-X240s (96.45.177.130.16clouds.com. [96.45.177.130]) by smtp.gmail.com with ESMTPSA id ke13sm10791624pjb.44.2021.02.20.00.11.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 20 Feb 2021 00:11:18 -0800 (PST) Date: Sat, 20 Feb 2021 16:11:10 +0800 From: Leo Yan To: James Clark Subject: Re: [PATCH 1/7] perf cs-etm: Split up etm queue setup function Message-ID: <20210220081110.GA3824@leoy-ThinkPad-X240s> References: <20210212144513.31765-1-james.clark@arm.com> <20210212144513.31765-2-james.clark@arm.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210212144513.31765-2-james.clark@arm.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210220_031123_931334_88C8F3EF X-CRM114-Status: GOOD ( 23.85 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , branislav.rankov@arm.com, al.grant@arm.com, denik@chromium.org, Mathieu Poirier , suzuki.poulose@arm.com, Alexander Shishkin , Will Deacon , coresight@lists.linaro.org, John Garry , linux-kernel@vger.kernel.org, Namhyung Kim , Jiri Olsa , linux-arm-kernel@lists.infradead.org, Mike Leach 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 Hi James, On Fri, Feb 12, 2021 at 04:45:07PM +0200, James Clark wrote: > Refactor the function into separate allocation and > timestamp search parts. Later the timestamp search > will be done multiple times. The new introduced function cs_etm__search_first_timestamp() is to search timestamp; if it cannot find any valid timestamp, it will drop all packets for every queue with the logic: cs_etm__search_first_timestamp() { timestamp = cs_etm__etmq_get_timestamp(); if (timestamp) return auxtrace_heap__add(); -> heapify timestamp cs_etm__clear_all_packet_queues(); -> clear all packets return 0; } If the function cs_etm__search_first_timestamp() is invoked for multiple times, is it possible to clear all packets in the middle of decoding? >From my understanding, it makes sense to drop the trace data at the very early decoding so that can get the timestamp for packets, afterwards the packets should always have valid timestamp? If so, I don't think it's necessary to contain the function cs_etm__clear_all_packet_queues() in cs_etm__search_first_timestamp(). I will go back to check this conclusion is correct or not after I have better understanding for the whole patch set. Just note here. Thanks, Leo > Signed-off-by: James Clark > --- > tools/perf/util/cs-etm.c | 60 +++++++++++++++++++++------------------- > 1 file changed, 31 insertions(+), 29 deletions(-) > > diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c > index a2a369e2fbb6..27894facae5e 100644 > --- a/tools/perf/util/cs-etm.c > +++ b/tools/perf/util/cs-etm.c > @@ -765,33 +765,12 @@ static struct cs_etm_queue *cs_etm__alloc_queue(struct cs_etm_auxtrace *etm) > return NULL; > } > > -static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > - struct auxtrace_queue *queue, > - unsigned int queue_nr) > +static int cs_etm__search_first_timestamp(struct cs_etm_queue *etmq) > { > int ret = 0; > + u64 timestamp; > unsigned int cs_queue_nr; > u8 trace_chan_id; > - u64 timestamp; > - struct cs_etm_queue *etmq = queue->priv; > - > - if (list_empty(&queue->head) || etmq) > - goto out; > - > - etmq = cs_etm__alloc_queue(etm); > - > - if (!etmq) { > - ret = -ENOMEM; > - goto out; > - } > - > - queue->priv = etmq; > - etmq->etm = etm; > - etmq->queue_nr = queue_nr; > - etmq->offset = 0; > - > - if (etm->timeless_decoding) > - goto out; > > /* > * We are under a CPU-wide trace scenario. As such we need to know > @@ -808,7 +787,7 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > */ > ret = cs_etm__get_data_block(etmq); > if (ret <= 0) > - goto out; > + return ret; > > /* > * Run decoder on the trace block. The decoder will stop when > @@ -817,7 +796,7 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > */ > ret = cs_etm__decode_data_block(etmq); > if (ret) > - goto out; > + return ret; > > /* > * Function cs_etm_decoder__do_{hard|soft}_timestamp() does all > @@ -849,10 +828,33 @@ static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > * Note that packets decoded above are still in the traceID's packet > * queue and will be processed in cs_etm__process_queues(). > */ > - cs_queue_nr = TO_CS_QUEUE_NR(queue_nr, trace_chan_id); > - ret = auxtrace_heap__add(&etm->heap, cs_queue_nr, timestamp); > -out: > - return ret; > + cs_queue_nr = TO_CS_QUEUE_NR(etmq->queue_nr, trace_chan_id); > + return auxtrace_heap__add(&etmq->etm->heap, cs_queue_nr, timestamp); > +} > + > +static int cs_etm__setup_queue(struct cs_etm_auxtrace *etm, > + struct auxtrace_queue *queue, > + unsigned int queue_nr) > +{ > + struct cs_etm_queue *etmq = queue->priv; > + > + if (list_empty(&queue->head) || etmq) > + return 0; > + > + etmq = cs_etm__alloc_queue(etm); > + > + if (!etmq) > + return -ENOMEM; > + > + queue->priv = etmq; > + etmq->etm = etm; > + etmq->queue_nr = queue_nr; > + etmq->offset = 0; > + > + if (etm->timeless_decoding) > + return 0; > + else > + return cs_etm__search_first_timestamp(etmq); > } > > static int cs_etm__setup_queues(struct cs_etm_auxtrace *etm) > -- > 2.28.0 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel