[PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration
Suzuki K Poulose
suzuki.poulose at arm.com
Thu Jul 16 05:42:26 PDT 2026
On 07/07/2026 10:54, Yeoreum Yun wrote:
> On Tue, Jul 07, 2026 at 09:59:36AM +0100, Yeoreum Yun wrote:
>> On Tue, Jul 07, 2026 at 09:25:10AM +0100, Leo Yan wrote:
>>> On Mon, Jul 06, 2026 at 07:16:42PM +0100, Yeoreum Yun wrote:
>>>
>>> [...]
>>>
>>>>> 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.
>>>
>>> My interpretation is:
>>>
>>> After this series, we updates active_config on local CPU within atomic
>>> context, it is nature to be protected. So we don't need to use
>>> feat_csdev->drv_spinlock when updating active_config.
>>>
>>> If so, it is fine for me to remove feat_csdev->drv_spinlock.
>>
>> Yes. but if I add a little bit more comment. the "active_config"
>> is accessed after "mode changed (perf of sysfs)" it accesses atomically
>> and the "active_config" is refered from the configfs not "config"
>> after this patch. So we can remove the "feat_csdev->drv_spinlock".
Ack
Suzuki
More information about the linux-arm-kernel
mailing list