From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 732FB436BC7 for ; Mon, 6 Jul 2026 18:16:47 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783361809; cv=none; b=ml8o+sm+ndU6vTI6G24CstLeDwo52eGWtmCvjDdTTu0wWKHKjDX+uV5UJSZw72VzuOUxtVUAjfqr1hjDVY3hCqQHoJDleon1BQEVNX9yskZ2hlfj3yBHQ7WeCoKZinL2E9+Ef6FQQTppCEVOjYVoSB4e7GxTAfy6BIdFg929kuI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783361809; c=relaxed/simple; bh=3sVK4y13JlgfB9hgXB3M941FZ0eD1FDI0MxT/6Nr71Y=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=uRJLBno0VkEmv41mOgt6ATW9dRvXzq1grNxR2WN1cI48NaN5fA+Ho2lMuY4RpBTeRqUp8dEgVMoJX0cUKn1GLMDYKOyTVeaiyWRvBkVbhMleN9RRUU19bYHLK/Q9PLo7uygGOZ2Zr/nm6RY7yX5WQPw2lFUlr9hTTijptC+g7WA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=Is6tvHJE; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="Is6tvHJE" 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 726BA1756; Mon, 6 Jul 2026 11:16:42 -0700 (PDT) Received: from e129823.arm.com (e129823.arm.com [10.2.213.3]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 65F133F905; Mon, 6 Jul 2026 11:16:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1783361806; bh=3sVK4y13JlgfB9hgXB3M941FZ0eD1FDI0MxT/6Nr71Y=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Is6tvHJExjwUdWhqiSia/hirSmhDfQXN/z56qupMbqyEMS2HTFcHQ4qoRBcgI6OsV ofPhbKjv+moDgH5nzPXv7yqz+R9TTbpwg1JFMqIKbyKNeCmn5xZPgnR76N41rPcWBe E+y15+09kUD4u7iSw/p4+eW0W5f4wBt+EhZSyNvo= Date: Mon, 6 Jul 2026 19:16:42 +0100 From: Yeoreum Yun To: Leo Yan Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, suzuki.poulose@arm.com, mike.leach@arm.com, james.clark@linaro.org, alexander.shishkin@linux.intel.com, jie.gan@oss.qualcomm.com Subject: Re: [PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration Message-ID: References: <20260629090007.1718746-1-yeoreum.yun@arm.com> <20260629090007.1718746-9-yeoreum.yun@arm.com> <20260706172159.GA1024232@e132581.arm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260706172159.GA1024232@e132581.arm.com> Hi Leo, > On Mon, Jun 29, 2026 at 10:00:01AM +0100, Yeoreum Yun wrote: > > [...] > > > @@ -619,27 +622,52 @@ static int etm4_enable_hw(struct etmv4_drvdata *drvdata) > > static void etm4_enable_sysfs_smp_call(void *info) > > { > > struct etm4_enable_arg *arg = info; > > + struct etmv4_drvdata *drvdata; > > struct coresight_device *csdev; > > + unsigned long cfg_hash; > > + int preset; > > > > if (WARN_ON(!arg)) > > return; > > > > - csdev = arg->drvdata->csdev; > > + drvdata = arg->drvdata; > > + csdev = drvdata->csdev; > > if (!coresight_take_mode(csdev, CS_MODE_SYSFS)) { > > /* Someone is already using the tracer */ > > arg->rc = -EBUSY; > > return; > > } > > > > - arg->rc = etm4_enable_hw(arg->drvdata); > > + drvdata->active_config = arg->config; > > > > - /* The tracer didn't start */ > > + /* enable any config activated by configfs */ > > + cscfg_config_sysfs_get_active_cfg(&cfg_hash, &preset); > > + if (cfg_hash) { > > + arg->rc = cscfg_csdev_enable_active_config(csdev, > > + cfg_hash, > > + preset); > > + if (arg->rc) > > + goto err; > > + } > > > I try best to not paste any non-sense AI reviews, record one from > Sashiko [1]: > > | If a user writes to a sysfs attribute like pe_sel_store(), it acquires > | drvdata->spinlock without disabling interrupts. If an IPI is then handled > | on the same CPU, etm4_enable_sysfs_smp_call() will call > | cscfg_csdev_enable_active_config() which attempts to acquire the exact > | same spinlock, hanging the CPU." > > The flow for acquiring drvdata->spinlock in SMP call is: > > etm4_enable_sysfs_smp_call() > `> cscfg_csdev_enable_active_config() > `> cscfg_csdev_enable_config() > `> cscfg_prog_config() > `> cscfg_set_on_enable() > `> raw_spin_lock_irqsave(feat_csdev->drv_spinlock, flags); > > We might expect the complaint from LOCKDEP with this patch. Hmm. I miss the this point and there is two solution: 1. Remove "feat_csdev->drv_spinlock" Before this patch, the "feat_csdev->drv_spinlock" is for synchronizing with the "drvdata->config". However, After this patch, the the configfs only show the "active" config and the active config can be used only after taking the "mode". Furthermore, the sysfs only uses the "config" not "active config", there is no race between the sysfs and the configfs. When I check the, "feat_csdev->drv_spinlock" is for the active/deactive the configfs config, so I seems safe to remove. 2. sysfs with _irqsave? However, this make a latency for the sysfs. I think the (1) seems good. Any comments? @Suzuki and @Leo? -- Sincerely, Yeoreum Yun