Re: [PATCH v2] coresight: configfs: Fix unload of configurations on module exit

Hi Mike,

Thanks for fixing this. Except for a minor nit, the patch looks good to me.

On 06/06/2022 16:26, Mike Leach wrote:

Any loaded configurations must be correctly unloaded on coresight module exit, or issues can arise with nested locking in the configfs directory code if built with CONFIG_LOCKDEP.

Prior to this patch, the preloaded configuration configfs directory entries were being unloaded by the recursive code in configfs_unregister_subsystem().

However, when built with CONFIG_LOCKDEP, this caused a nested lock warning, which was not mitigated by the LOCKDEP dependent code in fs/configfs/dir.c designed to prevent this, due to the different directory levels for the root of the directory being removed.

As the preloaded (and all other) configurations are registered after configfs_register_subsystem(), we now explicitly unload them before the call to configfs_unregister_subsystem().

The new routine cscfg_unload_cfgs_on_exit() iterates through the load owner list to unload any remaining configurations that were not unloaded by the user before the module exits. This covers both the CSCFG_OWNER_PRELOAD and CSCFG_OWNER_MODULE owner types, and will be extended to cover future load owner types for CoreSight configurations.

Applies to coresight/next

Fixes: eb2ec49606c2 ("coresight: syscfg: Update load API for config loadable modules") Reported-by: Suzuki Poulose suzuki.poulose@arm.com Signed-off-by: Mike Leach mike.leach@linaro.org

---

Changes since v1: Altered ordering of init of cscfg_mgr to ensure lists valid for potential exit path on error.

---

.../hwtracing/coresight/coresight-syscfg.c | 72 ++++++++++++++++--- 1 file changed, 61 insertions(+), 11 deletions(-)

diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 11850fd8c3b5..050a32f7e439 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -1042,6 +1042,13 @@ static int cscfg_create_device(void) if (!cscfg_mgr) goto create_dev_exit_unlock;

• /* initialise the cscfg_mgr structure */
• INIT_LIST_HEAD(&cscfg_mgr->csdev_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->feat_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->config_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->load_order_list);
• atomic_set(&cscfg_mgr->sys_active_cnt, 0);
• /* setup the device */ dev = cscfg_device(); dev->release = cscfg_dev_release;

@@ -1056,14 +1063,61 @@ static int cscfg_create_device(void) return err; } -static void cscfg_clear_device(void) +/*

• • Loading and unloading is generally on user discretion.
• • If exiting due to coresight module unload, we need to unload any configurations that remain,
• • before we unregister the configfs intrastructure.
• • Do this by walking the load_owner list and taking appropriate action, depending on the load
• • owner type.
• • called with the cscfg_mutex held
• */

+#define LOADABLE_MOD_ERR "cscfg: ERROR - a loadable module failed to unload configs on exit\n"

minor nit: Could we skip this ?

+static void cscfg_unload_cfgs_on_exit(void) {

• struct cscfg_config_desc *cfg_desc;

• struct cscfg_load_owner_info *owner_info = NULL;

• mutex_lock(&cscfg_mutex);
• list_for_each_entry(cfg_desc, &cscfg_mgr->config_desc_list, item) {

• etm_perf_del_symlink_cscfg(cfg_desc);
  

• while (!list_empty(&cscfg_mgr->load_order_list)) {

• /* remove in reverse order of loading */
  

• owner_info = list_last_entry(&cscfg_mgr->load_order_list,
  

• 			     struct cscfg_load_owner_info, item);
  

• /* action according to type */
  

• switch (owner_info->type) {
  

• case CSCFG_OWNER_PRELOAD:
  

• 	/*
  

• 	 * preloaded  descriptors are statically allocated in
  

• 	 * this module - just need to unload dynamic items from
  

• 	 * csdev lists, and remove from configfs directories.
  

• 	 */
  

• 	pr_info("cscfg: unloading preloaded configurations\n");
  

• 	cscfg_unload_owned_cfgs_feats(owner_info);
  

• 	break;
  

• case  CSCFG_OWNER_MODULE:
  

• 	/*
  

• 	 * this is an error - the loadable module must have been unloaded prior
  

• 	 * to the coresight module unload. Therefore that module has not
  

• 	 * correctly unloaded configs in its own exit code.
  

• 	 * Nothing to do other than emit an error string.
  

• 	 */
  

• 	pr_err(LOADABLE_MOD_ERR);
  

Instead : pr_err("cscfg: ERROR - a loadable module failed" " to unload configs on exit\n");

Otherwise, I can confirm that the patch fixes the reported problem.

• 	break;
  

• }
  

• /* remove from load order list */
  

• list_del(&owner_info->item);
  

  }

+}

+static void cscfg_clear_device(void) +{

• mutex_lock(&cscfg_mutex);
• cscfg_unload_cfgs_on_exit(); cscfg_configfs_release(cscfg_mgr); device_unregister(cscfg_device()); mutex_unlock(&cscfg_mutex);

@@ -1074,20 +1128,16 @@ int __init cscfg_init(void) { int err = 0;

• /* create the device and init cscfg_mgr */ err = cscfg_create_device(); if (err) return err;

• /* initialise configfs subsystem */ err = cscfg_configfs_init(cscfg_mgr); if (err) goto exit_err;

• INIT_LIST_HEAD(&cscfg_mgr->csdev_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->feat_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->config_desc_list);
• INIT_LIST_HEAD(&cscfg_mgr->load_order_list);
• atomic_set(&cscfg_mgr->sys_active_cnt, 0);
• /* preload built-in configurations */ err = cscfg_preload(THIS_MODULE); if (err)