From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) (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 1D5A9186E2E for ; Tue, 24 Dec 2024 11:41:46 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.46 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735040509; cv=none; b=uYknao5ONRknfWeZcv/rW9tzvNsDYM4LAQphHls1+U5I3EdSH5SjM9evQkQEpDSjS/UZa5NWPJyJ9wvc026/2kxBIYZRfQFSnzVs/ptxf+Ipi4PkOGxCZ3kG9Bb6tRwNLctOahtW/eQEKAQiiWvdW/rDw0J3t9o8TicQfL7ygSQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735040509; c=relaxed/simple; bh=F+77fED3Bn5kb21UFIpOXDBYlfDNJ7oU2qI2u5rz9+U=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=A67cuIhKNpq5/eglgXZlMsXvWy3beunUkbQmEHP1o75UsaBAvTYMZ8s60Ag8JNt0rdSglkbVokXFnsbm9GeyVIwTTcnbOCiVE8QPOufvSXcWm4Z/D2gVnZagrdBgcsomAMk2Um4gaQPEfX7SUymfmpRN8WuzelQ8W2ozkQcoxnQ= 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=WlKMXxm8; arc=none smtp.client-ip=209.85.221.46 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="WlKMXxm8" Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-388cae9eb9fso2831880f8f.3 for ; Tue, 24 Dec 2024 03:41:46 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1735040505; x=1735645305; darn=vger.kernel.org; h=content-transfer-encoding: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; bh=26RXT7+kDrUKYJZdXzBZ/W6d+4HGXX7y6cw0oURm71o=; b=WlKMXxm8Ao5FOfJImFmgfF0YtJAhas/Ahmvz8SgznJCYiLWfnCw6vaH7+z58NiRKmo 4vgo2FSPOBhjOG75gQ51uzZ2abLaQRbOAH8rLxbtZbF14mKKmGfhio4hseWudvh3uEVJ aqcUvMzUFpfhJ5TCT3frjn4Fm4gDlnMseuf/DX9475RkYvGDsZkvrSOT/WfpDmmBr5ie IObnDxuIR+oUQD3H2Wg5tHkCYZszjZaJMXpjnCAi7qRfUogeuVzxmAUtjPvM9SFNZa3g 6uJ4PvgUhNL/64ecjrn1vZD4iFWlcpGCUw60jvzkka/GzYW5mFJYYo/mAHnlbpwDK4Zo Hg2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1735040505; x=1735645305; h=content-transfer-encoding:in-reply-to:from:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=26RXT7+kDrUKYJZdXzBZ/W6d+4HGXX7y6cw0oURm71o=; b=GCrhHQ+WvwtZNYUD1eYg/PbUcXzthOY8MRImv5eERWyj1UakKHSn9/RdFW4uw0vKHp HXNdGZzRHa2Tf9tXfPfnOQsN3MJNCihMKTABh7jycQ3syWJpV6lvr1i3BPOFW1fdcNSm 4ghwHtYS8TAIN4YGewCESZNbHsKzr0vQDqNM0UaXUQlz73DMJNL15QXmgx3+J7IiDJ5C u6SjypapKmsQfBp2unST2F7c4nXpHj0a8b8Gr+t3JN2Fz0ovlUZjnxM099OlcnzGFXND Fkw5ZPleU7nmUKHRvoiiXo7relSqt+STDDoFKeY74kdtTr8CuaNiGNqTlsolFe+9GbZU Ggww== X-Forwarded-Encrypted: i=1; AJvYcCVehzAC8m71Z8YuXtdTTKLXtQlwszGvUdcRpaB1GJMYbo6fsuavE2XPZ3V0zZ9b+pZ0V4Qwv11xQR9LO7A=@vger.kernel.org X-Gm-Message-State: AOJu0YziglRMw6CSyZtoAXbmGD0DYy8QvCBjvBi+cl+ykDtKcPs1/Mnt kkD6WAGlPnnIXAM5duVm8u7dawbmvQQjD6Un4IQ1rZDCuqJGOBC8YAfc0htgDWs= X-Gm-Gg: ASbGncsvcVm/M5VMt9aMRqbiSB8KIiK9J2HfC/rDrxURLeVYK4i2+fG3Yp1sUe1Tfaz U4mVZfucWcps0fWoRs1yLDkVChHktSYLhBIfx5M3LNKCsVQDvreswdXAqOLz9vCbBrXtMVstpaj aiNlAQnbtlhBzwve95fDxra6tktpiW+dOMX9cZm6rjH/54bAiuwsVOJV1MlktHn37KhZIDGDXTZ 3dxGpOJiHH3nBG41WN/Me6hWAASrTn4rQ6TXoTN5GU/6Qz/y+IklXszrGl2ou9Bug== X-Google-Smtp-Source: AGHT+IHwyztgKd0z2u3jgbXnkAOo1ZLdfstQSQyPw5AjgaWpasVuihPk3mudTcZsXf30mhvcn9Eo+A== X-Received: by 2002:adf:8902:0:b0:38a:36a3:fec3 with SMTP id ffacd0b85a97d-38a36a3ff3fmr2368166f8f.0.1735040505005; Tue, 24 Dec 2024 03:41:45 -0800 (PST) Received: from [192.168.68.163] ([145.224.66.70]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-38a1c828a8dsm14055725f8f.2.2024.12.24.03.41.43 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 24 Dec 2024 03:41:44 -0800 (PST) Message-ID: <79e11033-181a-418a-a56f-068b9ca62f56@linaro.org> Date: Tue, 24 Dec 2024 11:41:43 +0000 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 1/1] coresight: prevent deactivate active config while enable the config To: Yeoreum Yun , Mike Leach Cc: coresight@lists.linaro.org, linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, nd@arm.com, suzuki.poulose@arm.com, alexander.shishkin@linux.intel.com References: <20241218084833.609876-1-yeoreum.yun@arm.com> <0efe4ef1-30f5-47f8-b818-79b4cfa0891f@linaro.org> Content-Language: en-US From: James Clark In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 24/12/2024 10:13 am, Yeoreum Yun wrote: > Hi James. >>> diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c >>> index a70c1454b410..dfa7dcbaf25d 100644 >>> --- a/drivers/hwtracing/coresight/coresight-syscfg.c >>> +++ b/drivers/hwtracing/coresight/coresight-syscfg.c >>> @@ -953,7 +953,8 @@ int cscfg_config_sysfs_activate(struct cscfg_config_desc *config_desc, bool acti >>> cscfg_mgr->sysfs_active_config = cfg_hash; >>> } else { >>> /* disable if matching current value */ >>> - if (cscfg_mgr->sysfs_active_config == cfg_hash) { >>> + if (cscfg_mgr->sysfs_active_config == cfg_hash && >>> + !atomic_read(&cscfg_mgr->sys_enable_cnt)) { >>> _cscfg_deactivate_config(cfg_hash); >> >> So is sys_enable_cnt a global value? If a fix is needed doesn't it need to >> be a per-config refcount? >> >> Say you have two active configs, sys_enable_cnt is now 2, how do you disable >> one without it always skipping here when the other config is enabled? > > Sorry to miss this one!. > Because when one configuration is enabled, > cscfg_mgr->sysfs_active_config becomes !NULL, so it wouldn't happen > there is no two activate configurations. so sys_enable_cnt wouldn't be > 2. > > > Maybe "sys_enabled" is a better name then. Count implies that it can be more than one. And the doc could be updated to say it's only ever 0 or 1. But what about my other point about enabled always being a subset of active? Can we not change "sys_active_cnt" to a more generic "refcount", then both activation and enabling steps increment that same refcount, because they are both technically users of the config. Then you can solve the problem without adding another separate counter. I think that's potentially easier to understand. Although the easiest is just locking every function with the mutex (or a spinlock if it also needs to be used from Perf). Obviously all these atomics are harder to get right than that, and this isn't performance sensitive in any way.