Hi Levi,
On Fri, May 02, 2025 at 11:34:17AM +0100, Yeoreum Yun wrote:
[...]
--- a/drivers/hwtracing/coresight/coresight-etm4x-core.c +++ b/drivers/hwtracing/coresight/coresight-etm4x-core.c @@ -1020,6 +1020,9 @@ static void etm4_disable_sysfs(struct coresight_device *csdev) smp_call_function_single(drvdata->cpu, etm4_disable_hw, drvdata, 1);
raw_spin_unlock(&drvdata->spinlock);
- cscfg_csdev_disable_active_config(csdev);
In general, we need to split changes into several patches if each addresses a different issue. From my understanding, the change above is to fix missing to disable config when disable Sysfs mode.
If so, could we use a seperate patch for this change?
It's not a differnt issue. Without this line, the active count wouldn't decrese and it raise another issue -- unloadable moudle for active_cnt :( So I think it should be included in this patch.
I read the code again and concluded the change above is not related to locking and would be a separate issue: when we close a Sysfs session, we need to disable a config on a CoreSight device. Could you clarify what is meaning "unloadable moudle for active_cnt"? I only saw "cscfg_mgr->sys_active_cnt" is used for module unloading, but have no clue why "active_cnt" impacts module unloading.
Yes. but it also related "by this patch". When the load config and "activate" configuration via sysfs, not only the cscfg_mgr->sys_active_cnt is increase but also "individual cscfg->active_cnt". This patch extends the meaning of "cscfg->active_cnt" includes "enable of configuraiton". so that cscfg_config_desc_put() prevent decrease "module reference" while holding individual active_cnt. That's why without this change, the "module reference" couldn't be decreased. The problem before this change is deactivation doesn't chekc cscfg->active_cnt but put module reference whenever it calls.
Thanks for explanation and it makes sense to me.
As we discussed, given this patch is relative big, let us divide into three small patches for easier review.
- The first patch is to address that the sysfs interface misses to call cscfg_csdev_disable_active_config() for disabling config.
- The second patch fixes the locking issue for "config_csdev_list". As the "config_csdev_list" is protected by cscfg_csdev_lock, the cscfg_remove_owned_csdev_configs() function should use lock for exclusive operations.
- The third patch is to fix reference counter of a config module. The patch adds cscfg_config_desc_get() and cscfg_config_desc_put() in the config enabling / disabling flow for acquiring module reference counter.
Thanks, Leo