On 15/09/2025 11:33, Leo Yan wrote:
The device mode is defined as local type. This type cannot promise SMP-safe access.
Change to atomic type and impose relax ordering, which ensures the SMP-safe synchronisation and the ordering between the mode setting and relevant operations.
Fixes: 22fd532eaa0c ("coresight: etm3x: adding operation mode for etm_enable()")
Do we really have an issue here ? I thought we modify the mode under spinlock. Also the fixes commit looks odd. The mode was added to the coresight device only recently. Prior to that, it was part of drvdata and was serialized by the respective spinlocks. So I don't see how this was an issue.
Suzuki
Signed-off-by: Leo Yan leo.yan@arm.com
include/linux/coresight.h | 25 +++++++++++-------------- 1 file changed, 11 insertions(+), 14 deletions(-)
diff --git a/include/linux/coresight.h b/include/linux/coresight.h index 6de59ce8ef8ca45c29e2f09c1b979eb7686b685f..3e5e5acd0c7fcde7d312d440da4355faaf682c7b 100644 --- a/include/linux/coresight.h +++ b/include/linux/coresight.h @@ -251,15 +251,11 @@ struct coresight_trace_id_map {
by @coresight_ops.- @access: Device i/o access abstraction for this device.
- @dev: The device entity associated to this component.
- @mode: This tracer's mode, i.e sysFS, Perf or disabled. This is
actually an 'enum cs_mode', but is stored in an atomic type.
This is always accessed through local_read() and local_set(),
but wherever it's done from within the Coresight device's lock,
a non-atomic read would also work. This is the main point of
synchronisation between code happening inside the sysfs mode's
coresight_mutex and outside when running in Perf mode. A compare
and exchange swap is done to atomically claim one mode or the
other.
- @mode: The device mode, i.e sysFS, Perf or disabled. This is actually
an 'enum cs_mode' but stored in an atomic type. Access is always
through atomic APIs, ensuring SMP-safe synchronisation between
racing from sysFS and Perf mode. A compare-and-exchange
operation is done to atomically claim one mode or the other.- @refcnt: keep track of what is in use. Only access this outside of the
device's spinlock when the coresight_mutex held and mode ==CS_MODE_SYSFS. Otherwise it must be accessed from inside the@@ -288,7 +284,7 @@ struct coresight_device { const struct coresight_ops *ops; struct csdev_access access; struct device dev;
- local_t mode;
- atomic_t mode; int refcnt; bool orphan; /* sink specific fields */
@@ -621,13 +617,14 @@ static inline bool coresight_is_percpu_sink(struct coresight_device *csdev) static inline bool coresight_take_mode(struct coresight_device *csdev, enum cs_mode new_mode) {
- return local_cmpxchg(&csdev->mode, CS_MODE_DISABLED, new_mode) ==
CS_MODE_DISABLED;
- int curr = CS_MODE_DISABLED;
- return atomic_try_cmpxchg_acquire(&csdev->mode, &curr, new_mode); }
static inline enum cs_mode coresight_get_mode(struct coresight_device *csdev) {
- return local_read(&csdev->mode);
- return atomic_read_acquire(&csdev->mode); }
static inline void coresight_set_mode(struct coresight_device *csdev, @@ -643,7 +640,7 @@ static inline void coresight_set_mode(struct coresight_device *csdev, WARN(new_mode != CS_MODE_DISABLED && current_mode != CS_MODE_DISABLED && current_mode != new_mode, "Device already in use\n");
- local_set(&csdev->mode, new_mode);
- atomic_set_release(&csdev->mode, new_mode); }
struct coresight_device *coresight_register(struct coresight_desc *desc);