Re: [PATCH v8 08/13] coresight: etm4x: fix inconsistencies with sysfs configuration

[Suzuki K Poulose <suzuki.poulose@arm.com>]

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