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=-5.4 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no 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 AF9D4C388F9 for ; Fri, 23 Oct 2020 10:34:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6645B2225F for ; Fri, 23 Oct 2020 10:34:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S462267AbgJWKeh (ORCPT ); Fri, 23 Oct 2020 06:34:37 -0400 Received: from foss.arm.com ([217.140.110.172]:48872 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S462260AbgJWKeh (ORCPT ); Fri, 23 Oct 2020 06:34:37 -0400 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 25102113E; Fri, 23 Oct 2020 03:34:37 -0700 (PDT) Received: from [10.57.13.45] (unknown [10.57.13.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 73AF63F66E; Fri, 23 Oct 2020 03:34:33 -0700 (PDT) Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() To: Peter Zijlstra Cc: Mathieu Poirier , Sai Prakash Ranjan , Mike Leach , Ingo Molnar , Arnaldo Carvalho de Melo , Mark Rutland , Alexander Shishkin , Jiri Olsa , Namhyung Kim , coresight@lists.linaro.org, Stephen Boyd , linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org References: <20201022113214.GD2611@hirez.programming.kicks-ass.net> <20201022150609.GI2611@hirez.programming.kicks-ass.net> <788706f2-0670-b7b6-a153-3ec6f16e0f2e@arm.com> <20201022212033.GA646497@xps15> <20201023073905.GM2611@hirez.programming.kicks-ass.net> <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com> <20201023094115.GR2611@hirez.programming.kicks-ass.net> From: Suzuki Poulose Message-ID: Date: Fri, 23 Oct 2020 11:34:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 In-Reply-To: <20201023094115.GR2611@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-GB Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org On 10/23/20 10:41 AM, Peter Zijlstra wrote: > On Fri, Oct 23, 2020 at 09:49:53AM +0100, Suzuki Poulose wrote: >> On 10/23/20 8:39 AM, Peter Zijlstra wrote: > >>> So then I don't understand the !->owner issue, that only happens when >>> the task dies, which cannot be concurrent with event creation. Are you >> >> Part of the patch from Sai, fixes this by avoiding the dereferencing >> after event creation (by caching it). But the kernel events needs >> fixing. > > I'm fundamentally failing here. Creating a link to the sink is strictly > event-creation time. Why would you ever need it again later? Later you > already have the sink setup. > Sorry for the lack of clarity here, and you are not alone unless you have drowned in the CoreSight topologies ;-) Typically current generation of systems have the following topology : CPU0 etm0 \ \ ________ / \ CPU1 / \ etm1 \ \ /------- sink0 CPU2 / etm2 \ / \ ________ / / CPU3 / etm3 i.e, Multiple ETMs share a sink. [for the sake of simplicity, I have used one sink. Even though there could be potential sinks (of different types), none of them are private to the ETMs. So, in a nutshell, a sink can be reached by multiple ETMs. ] Now, for a session : perf record -e cs_etm/sinkid=sink0/u workload We create an event per CPU (say eventN, which are scheduled based on the threads that could execute on the CPU. At this point we have finalized the sink0, and have allocated necessary buffer for the sink0. Now, when the threads are scheduled on the CPUs, we start the appropriate events for the CPUs. e.g, CPU0 sched -> workload:0 - > etm0->event0_start -> Turns all the components upto sink0, starting the trace collection in the buffer. Now, if another CPU, CPU1 starts tracing event1 for workload:1 thread, it will eventually try to turn ON the sink0.Since sink0 is already active tracing event0, we could allow this to go through and collect the trace in the *same hardware buffer* (which can be demuxed from the single AUX record using the TraceID in the packets). Please note that we do double buffering and hardware buffer is copied only when the sink0 is stopped (see below). But, if the event scheduled on CPU1 doesn't belong to the above session, but belongs to different perf session (say, perf record -e cs_etm/sinkid=sink0/u benchmark), we can't allow this to succeed and mix the trace data in to that of workload and thus fail the operation. In a nutshell, since the sinks are shared, we start the sink on the first event and keeps sharing the sink buffer with any event that belongs to the same session (using refcounts). The sink is only released for other sessions, when there are no more events in the session tracing on any of the ETMs. I know this is fundamentally a topology issue, but that is not something we can fix. But the situation is changing and we are starting to see systems with per-CPU sinks. Hope this helps. Suzuki 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=-5.4 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=no 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 A793CC388F9 for ; Fri, 23 Oct 2020 10:36:08 +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 1268520FC3 for ; Fri, 23 Oct 2020 10:36:07 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="SG1hyMeQ" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1268520FC3 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=glG0sfOgcuGASNFr/IG4+4zQZ+waumONSPiZFnrY19Y=; b=SG1hyMeQeHyeO++97MbIoPNSH JR0ih9Nd/QaR1veDxKaBp0EPGioWSJQY5cTWHuWzJQH+kLILnw/rfuhjVKHTIH/+lTL/pHXJamLaG L11qfCr3sBfLrF2kNPpl+q+mTywvZjGIqk3mDERGxtIimpAQATOD6A0Wha0cwfB0LTEgESIH9cHit /HOGPf+DCYpcacOwAXuX36ZgLbWl3qqeos+g8JzyAyjbxe+BKO2D+kNJNZTIDInjYEeCVkaHp9oVJ 1s42tzwLKN2eWkLG09HObfnzi9stKm4eeiL/4UtPKuaFmsPHAGe/UI+s4kB7aeXJz42dtmifQrJp6 72WcN4+1A==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kVuPC-0003mg-9v; Fri, 23 Oct 2020 10:34:42 +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 1kVuP9-0003la-Ln for linux-arm-kernel@lists.infradead.org; Fri, 23 Oct 2020 10:34:40 +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 25102113E; Fri, 23 Oct 2020 03:34:37 -0700 (PDT) Received: from [10.57.13.45] (unknown [10.57.13.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 73AF63F66E; Fri, 23 Oct 2020 03:34:33 -0700 (PDT) Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() To: Peter Zijlstra References: <20201022113214.GD2611@hirez.programming.kicks-ass.net> <20201022150609.GI2611@hirez.programming.kicks-ass.net> <788706f2-0670-b7b6-a153-3ec6f16e0f2e@arm.com> <20201022212033.GA646497@xps15> <20201023073905.GM2611@hirez.programming.kicks-ass.net> <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com> <20201023094115.GR2611@hirez.programming.kicks-ass.net> From: Suzuki Poulose Message-ID: Date: Fri, 23 Oct 2020 11:34:32 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.3.2 MIME-Version: 1.0 In-Reply-To: <20201023094115.GR2611@hirez.programming.kicks-ass.net> Content-Language: en-GB X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20201023_063439_861196_A19CE764 X-CRM114-Status: GOOD ( 24.29 ) 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 , Sai Prakash Ranjan , Mathieu Poirier , Alexander Shishkin , linux-arm-msm@vger.kernel.org, coresight@lists.linaro.org, linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo , Stephen Boyd , Ingo Molnar , Namhyung Kim , Jiri Olsa , linux-arm-kernel@lists.infradead.org, Mike Leach 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 10/23/20 10:41 AM, Peter Zijlstra wrote: > On Fri, Oct 23, 2020 at 09:49:53AM +0100, Suzuki Poulose wrote: >> On 10/23/20 8:39 AM, Peter Zijlstra wrote: > >>> So then I don't understand the !->owner issue, that only happens when >>> the task dies, which cannot be concurrent with event creation. Are you >> >> Part of the patch from Sai, fixes this by avoiding the dereferencing >> after event creation (by caching it). But the kernel events needs >> fixing. > > I'm fundamentally failing here. Creating a link to the sink is strictly > event-creation time. Why would you ever need it again later? Later you > already have the sink setup. > Sorry for the lack of clarity here, and you are not alone unless you have drowned in the CoreSight topologies ;-) Typically current generation of systems have the following topology : CPU0 etm0 \ \ ________ / \ CPU1 / \ etm1 \ \ /------- sink0 CPU2 / etm2 \ / \ ________ / / CPU3 / etm3 i.e, Multiple ETMs share a sink. [for the sake of simplicity, I have used one sink. Even though there could be potential sinks (of different types), none of them are private to the ETMs. So, in a nutshell, a sink can be reached by multiple ETMs. ] Now, for a session : perf record -e cs_etm/sinkid=sink0/u workload We create an event per CPU (say eventN, which are scheduled based on the threads that could execute on the CPU. At this point we have finalized the sink0, and have allocated necessary buffer for the sink0. Now, when the threads are scheduled on the CPUs, we start the appropriate events for the CPUs. e.g, CPU0 sched -> workload:0 - > etm0->event0_start -> Turns all the components upto sink0, starting the trace collection in the buffer. Now, if another CPU, CPU1 starts tracing event1 for workload:1 thread, it will eventually try to turn ON the sink0.Since sink0 is already active tracing event0, we could allow this to go through and collect the trace in the *same hardware buffer* (which can be demuxed from the single AUX record using the TraceID in the packets). Please note that we do double buffering and hardware buffer is copied only when the sink0 is stopped (see below). But, if the event scheduled on CPU1 doesn't belong to the above session, but belongs to different perf session (say, perf record -e cs_etm/sinkid=sink0/u benchmark), we can't allow this to succeed and mix the trace data in to that of workload and thus fail the operation. In a nutshell, since the sinks are shared, we start the sink on the first event and keeps sharing the sink buffer with any event that belongs to the same session (using refcounts). The sink is only released for other sessions, when there are no more events in the session tracing on any of the ETMs. I know this is fundamentally a topology issue, but that is not something we can fix. But the situation is changing and we are starting to see systems with per-CPU sinks. Hope this helps. Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel