[RFC PATCH 1/8] coresight: syscfg: Update API to allow dynamic load and unload
Mathieu Poirier
mathieu.poirier at linaro.org
Mon May 17 10:27:52 PDT 2021
On Mon, May 17, 2021 at 11:15:15AM -0600, Mathieu Poirier wrote:
> On Wed, May 12, 2021 at 10:17:45PM +0100, Mike Leach wrote:
> > Update the load API to permit the runtime loading and unloading of new
> > configurations and features.
> >
> > On load, configurations and features are tagged with a "load owner" that
> > is used to determine sets that were loaded together in a given API call.
> >
> > To unload the API uses the load owner to unload all elements previously
> > loaded by that owner.
> >
> > The API also uses records the order in which different owners loaded
> > their elements into the system. Later loading configurations can use
> > previously loaded features, creating load dependencies. Therefore unload
> > is enforced strictly in the reverse order to load.
>
> There seems to be something wrong with the first sentence - I think the word
> "uses" is extra. I found another one like that below.
>
> Otherwise this patch works. Despite the relative simplicity of what it does it
> took me two tries to go through it, probably because of how spread out the
> changes are. I don't think you could have done things differently though.
>
> Reviewed-by: Mathieu Poirier <mathieu.poirier at linaro.org>
>
> >
> > A load owner will be an additional loadable module, or a configuration
> > created or loaded via configfs.
> >
> > Signed-off-by: Mike Leach <mike.leach at linaro.org>
> > ---
> > .../coresight/coresight-cfg-preload.c | 9 +-
> > .../hwtracing/coresight/coresight-config.h | 9 +-
> > .../coresight/coresight-syscfg-configfs.c | 20 +++
> > .../coresight/coresight-syscfg-configfs.h | 2 +
> > .../hwtracing/coresight/coresight-syscfg.c | 154 +++++++++++++++++-
> > .../hwtracing/coresight/coresight-syscfg.h | 30 +++-
> > 6 files changed, 216 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-cfg-preload.c b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > index 751af3710d56..e237a4edfa09 100644
> > --- a/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > +++ b/drivers/hwtracing/coresight/coresight-cfg-preload.c
> > @@ -24,8 +24,13 @@ static struct cscfg_config_desc *preload_cfgs[] = {
> > NULL
> > };
> >
> > +static struct cscfg_load_owner_info preload_owner = {
> > + .type = CSCFG_OWNER_PRELOAD,
> > +};
> > +
> > /* preload called on initialisation */
> > -int cscfg_preload(void)
> > +int cscfg_preload(void *owner_handle)
> > {
> > - return cscfg_load_config_sets(preload_cfgs, preload_feats);
> > + preload_owner.owner_handle = owner_handle;
> > + return cscfg_load_config_sets(preload_cfgs, preload_feats, &preload_owner);
> > }
> > diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h
> > index 25eb6c632692..9bd44b940add 100644
> > --- a/drivers/hwtracing/coresight/coresight-config.h
> > +++ b/drivers/hwtracing/coresight/coresight-config.h
> > @@ -97,6 +97,8 @@ struct cscfg_regval_desc {
> > * @params_desc: array of parameters used.
> > * @nr_regs: number of registers used.
> > * @regs_desc: array of registers used.
> > + * @load_owner: handle to load owner for dynamic load and unload of features.
> > + * @fs_group: reference to configfs group for dynamic unload.
> > */
> > struct cscfg_feature_desc {
> > const char *name;
> > @@ -107,6 +109,8 @@ struct cscfg_feature_desc {
> > struct cscfg_parameter_desc *params_desc;
> > int nr_regs;
> > struct cscfg_regval_desc *regs_desc;
> > + void *load_owner;
> > + struct config_group *fs_group;
> > };
> >
> > /**
> > @@ -128,7 +132,8 @@ struct cscfg_feature_desc {
> > * @presets: Array of preset values.
> > * @event_ea: Extended attribute for perf event value
> > * @active_cnt: ref count for activate on this configuration.
> > - *
> > + * @load_owner: handle to load owner for dynamic load and unload of configs.
> > + * @fs_group: reference to configfs group for dynamic unload.
> > */
> > struct cscfg_config_desc {
> > const char *name;
> > @@ -141,6 +146,8 @@ struct cscfg_config_desc {
> > const u64 *presets; /* nr_presets * nr_total_params */
> > struct dev_ext_attribute *event_ea;
> > atomic_t active_cnt;
> > + void *load_owner;
> > + struct config_group *fs_group;
> > };
> >
> > /**
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> > index c547816b9000..345a62f1b728 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.c
> > @@ -334,9 +334,19 @@ int cscfg_configfs_add_config(struct cscfg_config_desc *config_desc)
> > if (IS_ERR(new_group))
> > return PTR_ERR(new_group);
> > err = configfs_register_group(&cscfg_configs_grp, new_group);
> > + if (!err)
> > + config_desc->fs_group = new_group;
> > return err;
> > }
> >
> > +void cscfg_configfs_del_config(struct cscfg_config_desc *config_desc)
> > +{
> > + if (config_desc->fs_group) {
> > + configfs_unregister_group(config_desc->fs_group);
> > + config_desc->fs_group = NULL;
> > + }
> > +}
> > +
> > static struct config_item_type cscfg_features_type = {
> > .ct_owner = THIS_MODULE,
> > };
> > @@ -358,9 +368,19 @@ int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc)
> > if (IS_ERR(new_group))
> > return PTR_ERR(new_group);
> > err = configfs_register_group(&cscfg_features_grp, new_group);
> > + if (!err)
> > + feat_desc->fs_group = new_group;
> > return err;
> > }
> >
> > +void cscfg_configfs_del_feature(struct cscfg_feature_desc *feat_desc)
> > +{
> > + if (feat_desc->fs_group) {
> > + configfs_unregister_group(feat_desc->fs_group);
> > + feat_desc->fs_group = NULL;
> > + }
> > +}
> > +
> > int cscfg_configfs_init(struct cscfg_manager *cscfg_mgr)
> > {
> > struct configfs_subsystem *subsys;
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> > index 7d6ffe35ca4c..ea1e54d29f7f 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg-configfs.h
> > @@ -41,5 +41,7 @@ int cscfg_configfs_init(struct cscfg_manager *cscfg_mgr);
> > void cscfg_configfs_release(struct cscfg_manager *cscfg_mgr);
> > int cscfg_configfs_add_config(struct cscfg_config_desc *config_desc);
> > int cscfg_configfs_add_feature(struct cscfg_feature_desc *feat_desc);
> > +void cscfg_configfs_del_config(struct cscfg_config_desc *config_desc);
> > +void cscfg_configfs_del_feature(struct cscfg_feature_desc *feat_desc);
> >
> > #endif /* CORESIGHT_SYSCFG_CONFIGFS_H */
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c
> > index 180202402eb5..ab5ec43a9dad 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg.c
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg.c
> > @@ -250,6 +250,13 @@ static int cscfg_check_feat_for_cfg(struct cscfg_config_desc *config_desc)
> > static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
> > {
> > int err;
> > + struct cscfg_feature_desc *feat_desc_exist;
> > +
> > + /* new feature must have unique name */
> > + list_for_each_entry(feat_desc_exist, &cscfg_mgr->feat_desc_list, item) {
> > + if (!strcmp(feat_desc_exist->name, feat_desc->name))
> > + return -EEXIST;
> > + }
> >
> > /* add feature to any matching registered devices */
> > err = cscfg_add_feat_to_csdevs(feat_desc);
> > @@ -267,6 +274,13 @@ static int cscfg_load_feat(struct cscfg_feature_desc *feat_desc)
> > static int cscfg_load_config(struct cscfg_config_desc *config_desc)
> > {
> > int err;
> > + struct cscfg_config_desc *config_desc_exist;
> > +
> > + /* new configuration must have a unique name */
> > + list_for_each_entry(config_desc_exist, &cscfg_mgr->config_desc_list, item) {
> > + if (!strcmp(config_desc_exist->name, config_desc->name))
> > + return -EEXIST;
> > + }
> >
> > /* validate features are present */
> > err = cscfg_check_feat_for_cfg(config_desc);
> > @@ -354,6 +368,72 @@ int cscfg_update_feat_param_val(struct cscfg_feature_desc *feat_desc,
> > return err;
> > }
> >
> > +static void cscfg_remove_owned_csdev_configs(struct coresight_device *csdev, void *load_owner)
> > +{
> > + struct cscfg_config_csdev *config_csdev, *tmp;
> > +
> > + if (list_empty(&csdev->config_csdev_list))
> > + return;
> > +
> > + list_for_each_entry_safe(config_csdev, tmp, &csdev->config_csdev_list, node) {
> > + if (config_csdev->config_desc->load_owner == load_owner)
> > + list_del(&config_csdev->node);
> > + }
> > +}
> > +
> > +static void cscfg_remove_owned_csdev_features(struct coresight_device *csdev, void *load_owner)
> > +{
> > + struct cscfg_feature_csdev *feat_csdev, *tmp;
> > +
> > + if (list_empty(&csdev->feature_csdev_list))
> > + return;
> > +
> > + list_for_each_entry_safe(feat_csdev, tmp, &csdev->feature_csdev_list, node) {
> > + if (feat_csdev->feat_desc->load_owner == load_owner)
> > + list_del(&feat_csdev->node);
> > + }
> > +}
> > +
> > +/*
> > + * removal is relatively easy - just remove from all lists, anything that
> > + * matches the owner. Memory for the descriptors will be managed by the owner,
> > + * memory for the csdev items is devm_ allocated with the individual csdev
> > + * devices.
> > + */
> > +static void cscfg_unload_owned_cfgs_feats(void *load_owner)
> > +{
> > + struct cscfg_config_desc *config_desc, *cfg_tmp;
> > + struct cscfg_feature_desc *feat_desc, *feat_tmp;
> > + struct cscfg_registered_csdev *csdev_item;
> > +
> > + /* remove from each csdev instance feature and config lists */
> > + list_for_each_entry(csdev_item, &cscfg_mgr->csdev_desc_list, item) {
> > + /*
> > + * for each csdev, check the loaded lists and remove if
> > + * referenced descriptor is owned
> > + */
> > + cscfg_remove_owned_csdev_configs(csdev_item->csdev, load_owner);
> > + cscfg_remove_owned_csdev_features(csdev_item->csdev, load_owner);
> > + }
> > +
> > + /* remove from the config descriptor lists */
> > + list_for_each_entry_safe(config_desc, cfg_tmp, &cscfg_mgr->config_desc_list, item) {
> > + if (config_desc->load_owner == load_owner) {
> > + cscfg_configfs_del_config(config_desc);
> > + etm_perf_del_symlink_cscfg(config_desc);
> > + list_del(&config_desc->item);
> > + }
> > + }
> > +
> > + /* remove from the feature descriptor lists */
> > + list_for_each_entry_safe(feat_desc, feat_tmp, &cscfg_mgr->feat_desc_list, item) {
> > + if (feat_desc->load_owner == load_owner) {
> > + cscfg_configfs_del_feature(feat_desc);
> > + list_del(&feat_desc->item);
> > + }
> > + }
> > +}
> > +
> > /**
> > * cscfg_load_config_sets - API function to load feature and config sets.
> > *
> > @@ -361,13 +441,22 @@ int cscfg_update_feat_param_val(struct cscfg_feature_desc *feat_desc,
> > * descriptors and load into the system.
> > * Features are loaded first to ensure configuration dependencies can be met.
> > *
> > + * To facilitate dynamic loading and unloading, features and configurations
> > + * have a "load_owner", to allow later unload by the same owner. An owner may
> > + * be a loadable module or configuration dynamically created via configfs.
> > + * As later loaded configurations can use earlier loaded features, creating load
> > + * dependencies, a load order list is maintained. Unload is strictly in the
> > + * reverse order to load.
> > + *
> > * @config_descs: 0 terminated array of configuration descriptors.
> > * @feat_descs: 0 terminated array of feature descriptors.
> > + * @owner_info: Information on the owner of this set.
> > */
> > int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
> > - struct cscfg_feature_desc **feat_descs)
> > + struct cscfg_feature_desc **feat_descs,
> > + struct cscfg_load_owner_info *owner_info)
> > {
> > - int err, i = 0;
> > + int err = 0, i = 0;
> >
> > mutex_lock(&cscfg_mutex);
> >
> > @@ -380,8 +469,10 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
> > if (err) {
> > pr_err("coresight-syscfg: Failed to load feature %s\n",
> > feat_descs[i]->name);
> > + cscfg_unload_owned_cfgs_feats(owner_info);
> > goto exit_unlock;
> > }
> > + feat_descs[i]->load_owner = owner_info;
> > i++;
> > }
> > }
> > @@ -396,18 +487,74 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs,
> > if (err) {
> > pr_err("coresight-syscfg: Failed to load configuration %s\n",
> > config_descs[i]->name);
> > + cscfg_unload_owned_cfgs_feats(owner_info);
> > goto exit_unlock;
> > }
> > + config_descs[i]->load_owner = owner_info;
> > i++;
> > }
> > }
> >
> > + /* add the load owner to the load order list */
> > + list_add_tail(&owner_info->item, &cscfg_mgr->load_order_list);
> > +
> > exit_unlock:
> > mutex_unlock(&cscfg_mutex);
> > return err;
> > }
> > EXPORT_SYMBOL_GPL(cscfg_load_config_sets);
> >
> > +/**
> > + * cscfg_unload_config_sets - unload a set of configurations be owner.
Shouldn't this be "by owner."?
> > + *
> > + * Dynamic unload of configuration and feature sets is done on the basis of
> > + * the load owner of that set. Later loaded configurations can depend on
> > + * features loaded earlier.
> > + *
> > + * Therefore, unload is only possible if:-
> > + * 1) no configurations are active.
> > + * 2) the set being unloaded was the last to be loaded to maintain dependencies.
> > + *
> > + * @owner_info: Information on owner for set being unloaded.
> > + */
> > +int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info)
> > +{
> > + int err = 0;
> > + struct cscfg_load_owner_info *load_list_item = NULL;
> > +
> > + mutex_lock(&cscfg_mutex);
> > +
> > + /* cannot unload if anything is active */
> > + if (atomic_read(&cscfg_mgr->sys_active_cnt)) {
> > + err = -EBUSY;
> > + goto exit_unlock;
> > + }
> > +
> > + /* cannot unload if not last loaded in load order */
> > + if (!list_empty(&cscfg_mgr->load_order_list)) {
> > + load_list_item = list_last_entry(&cscfg_mgr->load_order_list,
> > + struct cscfg_load_owner_info, item);
> > + if (load_list_item != owner_info)
> > + load_list_item = NULL;
> > + }
> > +
> > + if (!load_list_item) {
> > + err = -EINVAL;
> > + goto exit_unlock;
> > + }
> > +
> > + /* unload all belonging to load_owner */
> > + cscfg_unload_owned_cfgs_feats(owner_info);
> > +
> > + /* remove from load order list */
> > + list_del(&load_list_item->item);
> > +
> > +exit_unlock:
> > + mutex_unlock(&cscfg_mutex);
> > + return err;
> > +}
> > +EXPORT_SYMBOL_GPL(cscfg_unload_config_sets);
> > +
> > /* Handle coresight device registration and add configs and features to devices */
> >
> > /* iterate through config lists and load matching configs to device */
> > @@ -783,10 +930,11 @@ int __init cscfg_init(void)
> > 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();
> > + err = cscfg_preload(THIS_MODULE);
> > if (err)
> > goto exit_err;
> >
> > diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h
> > index 8d018efd6ead..e2b2bdab31aa 100644
> > --- a/drivers/hwtracing/coresight/coresight-syscfg.h
> > +++ b/drivers/hwtracing/coresight/coresight-syscfg.h
> > @@ -25,6 +25,7 @@
> > * @csdev_desc_list: List of coresight devices registered with the configuration manager.
> > * @feat_desc_list: List of feature descriptors to load into registered devices.
> > * @config_desc_list: List of system configuration descriptors to load into registered devices.
> > + * @load_order_list: Ordered list of owners for dynamically loaded configurations.
> > * @sys_active_cnt: Total number of active config descriptor references.
> > * @cfgfs_subsys: configfs subsystem used to manage configurations.
> > */
> > @@ -33,6 +34,7 @@ struct cscfg_manager {
> > struct list_head csdev_desc_list;
> > struct list_head feat_desc_list;
> > struct list_head config_desc_list;
> > + struct list_head load_order_list;
> > atomic_t sys_active_cnt;
> > struct configfs_subsystem cfgfs_subsys;
> > };
> > @@ -56,10 +58,32 @@ struct cscfg_registered_csdev {
> > struct list_head item;
> > };
> >
> > +/* owner types for loading and unloading of config and feature sets */
> > +enum cscfg_load_owner_type {
> > + CSCFG_OWNER_PRELOAD,
> > +};
> > +
> > +/**
> > + * Load item - item to add to the load order list allowing dynamic load and
> > + * unload of configurations and features. Caller loading a config
> > + * set provides a context handle for unload. API ensures that
> > + * items unloaded strictly in reverse order from load to ensure
> > + * dependencies are respected.
> > + *
> > + * @item: list entry for load order list.
> > + * @type: type of owner - allows interpretation of owner_handle.
> > + * @owner_handle: load context - handle for owner of loaded configs.
> > + */
> > +struct cscfg_load_owner_info {
> > + struct list_head item;
> > + int type;
> > + void *owner_handle;
> > +};
> > +
> > /* internal core operations for cscfg */
> > int __init cscfg_init(void);
> > void cscfg_exit(void);
> > -int cscfg_preload(void);
> > +int cscfg_preload(void *owner_handle);
> > const struct cscfg_feature_desc *cscfg_get_named_feat_desc(const char *name);
> > int cscfg_update_feat_param_val(struct cscfg_feature_desc *feat_desc,
> > int param_idx, u64 value);
> > @@ -67,7 +91,9 @@ int cscfg_update_feat_param_val(struct cscfg_feature_desc *feat_desc,
> >
> > /* syscfg manager external API */
> > int cscfg_load_config_sets(struct cscfg_config_desc **cfg_descs,
> > - struct cscfg_feature_desc **feat_descs);
> > + struct cscfg_feature_desc **feat_descs,
> > + struct cscfg_load_owner_info *owner_info);
> > +int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info);
> > int cscfg_register_csdev(struct coresight_device *csdev, u32 match_flags,
> > struct cscfg_csdev_feat_ops *ops);
> > void cscfg_unregister_csdev(struct coresight_device *csdev);
> > --
> > 2.17.1
> >
More information about the linux-arm-kernel
mailing list