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=-15.3 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,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 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 F1F12C2D0E4 for ; Fri, 27 Nov 2020 10:33:53 +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 6D04420885 for ; Fri, 27 Nov 2020 10:33:53 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="kgXBPpw8" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 6D04420885 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=arm.com 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-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=HjJBnkAFvgYRB3wQmuE8iYqXruFF2vyuSzEziULinfY=; b=kgXBPpw8OmU8WeiO6wq3ooWUT Q0dX6tX7RYUfI7iCMFRqnVLwdB5zFlJysVrTbvd/deLSAlf9ZwtNC2snfxGos5VHTZ1boXkXIgWId d7PQCuJldrup0dJbL8Lm2mwQNovrH+KCQfs96LhnVw0CcfwFwQa4VC3shLr1TDmiDkTtrGoxjzstJ NJhET25X5kOU1lmhLmCosutLKSlloBpKfUBGWbSAEUcHUYYly6a553Iu0DcsCDjriRGytl/aYTPZV KbMIDXP9ExCIV4RthHWxE2Grn+vGVF2nGsys2D+lZ+/ZYdzT2Yz/2kA9gSTaulpfUqteEres/o0GD liVaxAclg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kib3T-0007tJ-4t; Fri, 27 Nov 2020 10:32:43 +0000 Received: from foss.arm.com ([217.140.110.172]) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kib3P-0007rp-L0 for linux-arm-kernel@lists.infradead.org; Fri, 27 Nov 2020 10:32:41 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 8CDDF1516; Fri, 27 Nov 2020 02:32:36 -0800 (PST) Received: from [10.57.56.151] (unknown [10.57.56.151]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 18CE03F71F; Fri, 27 Nov 2020 02:32:33 -0800 (PST) Subject: Re: [RFC 09/11] coresight: etm-perf: Disable the path before capturing the trace data To: Anshuman Khandual , linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org References: <1605012309-24812-1-git-send-email-anshuman.khandual@arm.com> <1605012309-24812-10-git-send-email-anshuman.khandual@arm.com> From: Suzuki K Poulose Message-ID: <2019e06d-65e1-fee8-f75d-bfa5750d2458@arm.com> Date: Fri, 27 Nov 2020 10:32:28 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.5.0 MIME-Version: 1.0 In-Reply-To: <1605012309-24812-10-git-send-email-anshuman.khandual@arm.com> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201127_053239_827594_CD12B100 X-CRM114-Status: GOOD ( 25.18 ) 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: Al Grant , linux-kernel@vger.kernel.org, mathieu.poirier@linaro.org, mike.leach@linaro.org Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 11/10/20 12:45 PM, Anshuman Khandual wrote: > perf handle structure needs to be shared with the TRBE IRQ handler for > capturing trace data and restarting the handle. There is a probability > of an undefined reference based crash when etm event is being stopped > while a TRBE IRQ also getting processed. This happens due the release > of perf handle via perf_aux_output_end(). This stops the sinks via the > link before releasing the handle, which will ensure that a simultaneous > TRBE IRQ could not happen. Or in other words : We now have : update_buffer() perf_aux_output_end(handle) ... disable_path() This is problematic due to various reasons : 1) The semantics of update_buffer() is not clear. i.e, whether it should leave the "sink" "stopped" or "disabled" or "active" 2) This breaks the recommended trace collection sequence of "flush" and "stop" from source to the sink for trace collection. i.e, we stop the source now. But don't flush the components from source to sink, rather we stop and flush from the sink. And we flush and stop the path after we have collected the trace data at sink, which is pointless. 3) For a sink with IRQ handler, if we don't stop the sink with update_buffer(), we could have a situation : update_buffer() perf_aux_outpuf_end(handle) # handle is invalid now -----------------> IRQ -> irq_handler() perf_aux_output_end(handle) # Wrong ! disable_path() The sysfs mode is fine, as we defer the trace collection to disable_path(). The proposed patch is still racy, as we could still hit the problem. So, to avoid all of these situations, I think we should defer the the update_buffer() to sink_ops->disable(), when we have flushed and stopped the all the components upstream and avoid any races with the IRQ handler. i.e, source_ops->stop(csdev); disable_path(handle); // similar to the enable_path sink_ops->disable(csdev, handle) { /* flush & stop */ /* collect trace */ perf_aux_output_end(handle, size); } Kind regards Suzuki > > Signed-off-by: Anshuman Khandual > --- > This might cause problem with traditional sink devices which can be > operated in both sysfs and perf mode. This needs to be addressed > correctly. One option would be to move the update_buffer callback > into the respective sink devices. e.g, disable(). > > drivers/hwtracing/coresight/coresight-etm-perf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/hwtracing/coresight/coresight-etm-perf.c b/drivers/hwtracing/coresight/coresight-etm-perf.c > index 534e205..1a37991 100644 > --- a/drivers/hwtracing/coresight/coresight-etm-perf.c > +++ b/drivers/hwtracing/coresight/coresight-etm-perf.c > @@ -429,7 +429,9 @@ static void etm_event_stop(struct perf_event *event, int mode) > > size = sink_ops(sink)->update_buffer(sink, handle, > event_data->snk_config); > + coresight_disable_path(path); > perf_aux_output_end(handle, size); > + return; > } > > /* Disabling the path make its elements available to other sessions */ > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel