From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f43.google.com (mail-wm1-f43.google.com [209.85.128.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B823539656C for ; Thu, 13 Aug 2026 09:25:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786613102; cv=none; b=FDkB2NPL32biK4TZBj1g8d+3h7fyQMS19oQXMZ3Sv7ihMQcHmhW+B/q8xSZIU4FvgES/ntfMFz0OwmqmMKeaRTl8A4K4ir+fEIUa8HRRNfnOlhzvJMcAUTtZIowIO4GpF5VhDSHCDcNQjSx4c+9GkZp+9EhGCv16OzDE7Dvtfb4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786613102; c=relaxed/simple; bh=2WiZLKDrfPnYyJBKxciRfcvhbkB4b5rfA3K+DXXht7U=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GOWgg7cgVoDCQbUvIGkvdfz6CuunaoChVeEbyDPDBDPF2JhCot4xdAH30b/xxrFnBhpHLq3M0fcGk/Fp4xVd05EBGFDxXnEsQfcvUwJUwBZ1B3/9uqrWjNbRooToOFoeXU7Ee5hk8EUpVfAy0Kzbh6hKTsbrvrjnoYdu1AUBJgc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org; spf=pass smtp.mailfrom=linaro.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b=nIlGsgbQ; arc=none smtp.client-ip=209.85.128.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linaro.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="nIlGsgbQ" Received: by mail-wm1-f43.google.com with SMTP id 5b1f17b1804b1-49556f97a9dso11567765e9.1 for ; Thu, 13 Aug 2026 02:25:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1786613099; x=1787217899; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=mR+ypn1dGm0BnehOIl3Z6EwsPWd7IoHNbFBDJMO2TcQ=; b=nIlGsgbQ6FIsluCBJTuaDUZ00YD8FnQ5JtTzZ2pz0MrLBS/j1Yj2xIRWyrJmxokd2k pmcz0EwS6DbmvUm1VTd8vnzsw8cBkInexwab9SvOLJdpmipD5RpdYULGZ5mygH7YS9bm o252pk6evzXDA3cFUXSYSHWm5lUeHyqXXgergljiSY7OkZI+D6jEfNCmJFlBMa4V2GdB kjljAGjlB2vGtJUEBNvKB+kehmxms9TuKRra2mvHTEGgdF8yO6Vzuc90yDnC7+Orsf3a aW2u3aPecgL8Xrbp4jT7Ynj7o9WW8R+urLQV/QrndQG+YDunILDv0zkv92Fx+3qKCMhp 8p1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786613099; x=1787217899; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=mR+ypn1dGm0BnehOIl3Z6EwsPWd7IoHNbFBDJMO2TcQ=; b=mWSp2neCFZ3UxueOUZWuSzq/nhZ37dli5fQKD4M+LNIzk+gpzs/M96bcJmQy40UIR6 LkMpW5VGgONALYTWUuRRluTJDF2/yXXUwf8cLLtFhHwO31Ci0nR69x3jaCZreCxMIVXC /TZjdMBWggr29LXAw8LoazWI0VdhWalAKHOAzmjNKYh5qLvUYAQf+WS41UY0B+56pc0R DTEDOrt1L+k5HbX8Qc+HFssaiy8hr8Lo59J7ZhVHEzxtpZAS30VK/pQ9BbDwK5wsXafa 5l8pUCHkgWMLNQn+Yh+Unt21g/6QIL5+DoCbdyUZFuJ5rd24DqXXf8KCJhwmgDOw9XVu Zqog== X-Forwarded-Encrypted: i=1; AHgh+RpmgfN795dnAtX5cTA23sl8v8P8/UmLIFtrcC2XBlafYGYGEQC5DC3jy3gjHrzLeaIVShCb7+RkMAuWApc=@vger.kernel.org X-Gm-Message-State: AOJu0Yz1aCaHQDL1expJgsMWzUertb9rtfPuNM0D1iK/+JXkLzLGwmPx iGI/bjA8qmJTIa3bApUP4PffrcgMwpbPOUi1ycOy2zPTx5iE+qSrI+0I5SvGtIliokE= X-Gm-Gg: AR+sD101GSFuEBovr9TQ+AxqcN3lIV7xqG6POdsLqQgBcDgfUs7HHz3wGfYbp/PSV1W 0dHTk5bHNMzWLHhYYQNkX8Juja7wSlfebzsqWfQ9iL1gFodD+DBJ5SbTfuxKSejIsBxzNk0wOK5 jl3fjIpwwGomhstt47LItARPB5jXfDcZeY+I7q4SOGdVC8daw7XGkkTxGIX3UjaS1jpYj6I4cui 9XpNzuoWpV7O0XQU3J18L351FFJLJoYSNeudxKjoOAePv/lEvmDLyz/ZP+/p5D76WJsZJ6yepwf +F2WSaw/UrAcmhONIuUKLLQiNEpfbBR4WzeJ6pnXNDnGxoudkTjDYEIrmsM10TGnqEpMFuL8wld NdaIdtMnZrILWjDRaki2qGdmlWV6qbo+BCKhwjEFrZK8sEJXNXY/rIaRYTsFkcC+u9UK3pytqhz MxuMuI3CGI5Roy2VwIqU7VpzuC3FXg1ttOpaNBB9+IwK6YMkd68Nirix19RyhrTjpxYBhIuPXVU YacDYYxZBrqHMqu X-Received: by 2002:a05:600c:8b2f:b0:495:6a50:3fb8 with SMTP id 5b1f17b1804b1-4998219cf7cmr56618815e9.1.1786613098945; Thu, 13 Aug 2026 02:24:58 -0700 (PDT) Received: from [192.168.1.3] ([37.18.141.193]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-49982131261sm82274035e9.8.2026.08.13.02.24.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 13 Aug 2026 02:24:58 -0700 (PDT) Message-ID: <9444ef3e-1159-4e23-bd71-716582f377a5@linaro.org> Date: Thu, 13 Aug 2026 10:24:56 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 2/8] coresight: configfs: Don't assume active until cscfg_mgr is set To: Leo Yan Cc: Suzuki K Poulose , Mike Leach , Suyash Mahar , Yeoreum Yun , Greg Kroah-Hartman , Qi Liu , Junhao He , coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Jonathan Cameron , sashiko-bot References: <20260728-james-cs-multiple-per-threads-v3-0-6aee7579f1dc@linaro.org> <20260728-james-cs-multiple-per-threads-v3-2-6aee7579f1dc@linaro.org> <20260813090924.GA8904@e132581.arm.com> Content-Language: en-US From: James Clark In-Reply-To: <20260813090924.GA8904@e132581.arm.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 13/08/2026 10:09, Leo Yan wrote: > On Tue, Jul 28, 2026 at 04:00:14PM +0100, James Clark wrote: >> Perf's etm_setup_aux() calls cscfg_activate_config() and dereferences >> cscfg_mgr, which can be done after coresight_init() has initialized the >> Perf PMU but before cscfg_init() has finished while the driver is still >> loading. > > For this patch, if Perf PMU is dependent on cscfg, why we cannot > adjust the init sequence that first call cscfg_init(), then register > cs_etm PMU event? > cscfg_init() relies on perf making some files in sysfs. I think I tried that first but it doesn't work: /* add config to perf fs to allow selection */ err = etm_perf_add_symlink_cscfg(cscfg_device(), config_desc); One solution would be to combine the modules into one. Not sure if there is any functional requirement for them being separate. They're both "software" parts of the set of Coresight modules, rather than being associated with any hardware. Having them separate is obviously creating some bugs and needless complexity. > In _cscfg_activate_config(), I can see it dereferences cscfg_mgr: > > if (cscfg_mgr->load_state == CSCFG_UNLOAD) > > This patch does not check if cscfg_mgr is set. If we try to check > cscfg_mgr every time before dereferences it, this approach is quite > fragile. > Maybe we can have a "cscfg_get_state()" function which also checks for NULL and returns CSCFG_UNLOAD in that case? > >> Fix it by only assuming cscfg is initialized if cscfg_mgr is set. >> >> Reported-by: sashiko-bot >> Fixes: a0114b4740dd ("coresight: etm-perf: Update to activate selected configuration") >> Signed-off-by: James Clark >> --- >> drivers/hwtracing/coresight/coresight-syscfg.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c >> index 2bfdd7b45e49..475e8fefa100 100644 >> --- a/drivers/hwtracing/coresight/coresight-syscfg.c >> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c >> @@ -581,7 +581,7 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs, >> int err = 0; >> >> mutex_lock(&cscfg_mutex); >> - if (cscfg_mgr->load_state != CSCFG_NONE) { >> + if (!cscfg_mgr || cscfg_mgr->load_state != CSCFG_NONE) { >> mutex_unlock(&cscfg_mutex); >> return -EBUSY; > > If cscfg_mgr is NULL, would directly bail out? This can avoid adding > check for every dereference. Not sure what you mean by this? > > Thanks, > Leo > > P.S. For me, this kind of rare race condition reported by Sashiko can > sometimes distract from the main issue. Trying to address it in the > same series may mix different issues together. I miss the simpler > approach where issues could be resolved with clear boundary. It's a bit like the other fix, we're holding new refcounts to the task_struct and PID, so we should probably make sure we're going to release them. Getting this wrong prevents free_event_data() from being called. Don't worry I didn't fix all of the edge cases either.