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

[Suzuki K Poulose]

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