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 CD672C55178 for ; Fri, 23 Oct 2020 08:50:02 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7FEC220795 for ; Fri, 23 Oct 2020 08:50:02 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S460767AbgJWIuB (ORCPT ); Fri, 23 Oct 2020 04:50:01 -0400 Received: from foss.arm.com ([217.140.110.172]:46708 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S460766AbgJWIuA (ORCPT ); Fri, 23 Oct 2020 04:50:00 -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 5541D31B; Fri, 23 Oct 2020 01:50:00 -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 99E4E3F66E; Fri, 23 Oct 2020 01:49:55 -0700 (PDT) Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() To: Peter Zijlstra , Mathieu Poirier Cc: 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> From: Suzuki Poulose Message-ID: <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com> Date: Fri, 23 Oct 2020 09:49:53 +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: <20201023073905.GM2611@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 Hi Peter On 10/23/20 8:39 AM, Peter Zijlstra wrote: > On Thu, Oct 22, 2020 at 03:20:33PM -0600, Mathieu Poirier wrote: >> Suzuki's depiction of the usecase is accurate. Using the pid of the process >> that created the events comes out of a discussion you and I had in the common >> area by the Intel booth at ELC in Edinburgh in the fall of 2018. At the time I >> exposed the problem of having multiple events sharing the same HW resources and >> you advised to proceed this way. > > Bah, I was afraid of that. I desperately tried to find correspondence on > it, but alas, verbal crap doesn't end up in the Sent folder :-/ > >> That being said it is plausible that I did not expressed myself clearly enough >> for you to understand the full extend of the problem. If that is the case we >> are more than willing to revisit that solution. Do you see a better option than >> what has currently been implemented? > > Moo... that really could've done with a comment I suppose. > > 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. One follow up question on the !->owner issue. Given the ->owner is dying, does it prevent events from being scheduled ? Or is there a delay between that and eventually stopping the events. In this case, we hit the issue when : A A or B ? event_start() ... event->owner = NULL READ_ONCE(event->owner); Is this expected ? > somehow accessing ->owner later? > > As for the kernel events.. why do you care about the actual task_struct > * in there? I see you're using it to grab the task-pid, but how is that > useful? Correct, kernel events are something that the driver didn't account for. May be we could handle this case with a "special pid" and simply disallow sharing (which is fine I believe, given there are not grouping for the kernel created events). 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 DC3F8C55178 for ; Fri, 23 Oct 2020 08:51:28 +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 5FB902192A for ; Fri, 23 Oct 2020 08:51:28 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="TgxkIzYV" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5FB902192A 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=X4HRI6DR7BxJEmuJwHxfkgBSW+WQqD+sccwPLCUL7u8=; b=TgxkIzYVmCnw+QmiOxN6vEber LS4+ogLNkBr81z8olRC/FfMxvU+eaShguDKgvPniI5Q6bRImQueamL01r6pUkAPhqeAf8dKHUr6qY QVytMHbWvNCCxxKCnwYmZNy4BNx/4Y2jMdtrijLWpxN9/EJ3NQoZpdyk77pB2PfVMvWVa56bEO2y0 SZcTthFGBPG+R89Iif+1IBN1a9rIj9PDspZohWdvKwb3lvY5fYMArAYnXssCtuvR30GcokT6ZvXSu c9plg72QbyrLxUEstVDPTESC6qUP5rapV+CHokRYTpnO9wKf+piW1b4j9S9KtRmCNStRZ8x9N5J0W 86flvlDXg==; Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1kVsm1-0004aM-RF; Fri, 23 Oct 2020 08:50:09 +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 1kVsly-0004Z2-CD for linux-arm-kernel@lists.infradead.org; Fri, 23 Oct 2020 08:50:07 +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 5541D31B; Fri, 23 Oct 2020 01:50:00 -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 99E4E3F66E; Fri, 23 Oct 2020 01:49:55 -0700 (PDT) Subject: Re: [PATCHv2 2/4] coresight: tmc-etf: Fix NULL ptr dereference in tmc_enable_etf_sink_perf() To: Peter Zijlstra , Mathieu Poirier 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> From: Suzuki Poulose Message-ID: <174e6461-4d46-cb65-c094-c06ee3b21568@arm.com> Date: Fri, 23 Oct 2020 09:49:53 +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: <20201023073905.GM2611@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_045006_613340_DE549E3A X-CRM114-Status: GOOD ( 27.42 ) 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 , 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 Hi Peter On 10/23/20 8:39 AM, Peter Zijlstra wrote: > On Thu, Oct 22, 2020 at 03:20:33PM -0600, Mathieu Poirier wrote: >> Suzuki's depiction of the usecase is accurate. Using the pid of the process >> that created the events comes out of a discussion you and I had in the common >> area by the Intel booth at ELC in Edinburgh in the fall of 2018. At the time I >> exposed the problem of having multiple events sharing the same HW resources and >> you advised to proceed this way. > > Bah, I was afraid of that. I desperately tried to find correspondence on > it, but alas, verbal crap doesn't end up in the Sent folder :-/ > >> That being said it is plausible that I did not expressed myself clearly enough >> for you to understand the full extend of the problem. If that is the case we >> are more than willing to revisit that solution. Do you see a better option than >> what has currently been implemented? > > Moo... that really could've done with a comment I suppose. > > 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. One follow up question on the !->owner issue. Given the ->owner is dying, does it prevent events from being scheduled ? Or is there a delay between that and eventually stopping the events. In this case, we hit the issue when : A A or B ? event_start() ... event->owner = NULL READ_ONCE(event->owner); Is this expected ? > somehow accessing ->owner later? > > As for the kernel events.. why do you care about the actual task_struct > * in there? I see you're using it to grab the task-pid, but how is that > useful? Correct, kernel events are something that the driver didn't account for. May be we could handle this case with a "special pid" and simply disallow sharing (which is fine I believe, given there are not grouping for the kernel created events). Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel