From patchwork Fri Jun 17 16:40:08 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mike Leach X-Patchwork-Id: 12885789 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 442A3C43334 for ; Fri, 17 Jun 2022 16:41:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:MIME-Version:List-Subscribe:List-Help: List-Post:List-Archive:List-Unsubscribe:List-Id:References:In-Reply-To: Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=DiyxKuMlKhmFC6Dsm73lITyuZ8NxTzkYC8MYGhsTCU8=; b=nley3TJGp5mSDb GNs6G4sHZv6hSrjCPagPRKFH5itu65kESTQcyL2ATnMDKs7/UmDRBHFbErKIjk1+2JwZ2TOeTt7Hm qlCNTxVHA881yRclTBqGCjFKloL2ExedIyCRJSOrS3VrIuKuzSuLALPuoHxOjx6lVlTNOTuoJEmu1 iHCZjDZXokQvMZLewyLrDGfOKrwkbojGajBGqyJ5Q/+rF94KgtCe125Oid6eo6JeXCTBX5kIN3cA8 f7b7xGZUf2tv1BL6/coaXnhhOHx/2Et7IfTemC6IZmShl0OZCOCruaCVqVTzAct2rZqg7YU7y9VMh 4Ls0mtKX1KxTDLZ55M7w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1o2F1Q-008GHC-Ik; Fri, 17 Jun 2022 16:40:36 +0000 Received: from mail-wm1-x329.google.com ([2a00:1450:4864:20::329]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1o2F17-008G8u-FP for linux-arm-kernel@lists.infradead.org; Fri, 17 Jun 2022 16:40:19 +0000 Received: by mail-wm1-x329.google.com with SMTP id q15so2595123wmj.2 for ; Fri, 17 Jun 2022 09:40:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=Im4T5nLORSBFzIzLAcJIJ7W14tpegoLQvSTH9D23aeU=; b=N4vHS8pUEkevhawfmC5ZXESeWao/GQt+zNjXdroDPSOdo68A6xJjMh9byuJTqUDJ2Y 6vgtfZ8ThyMKN6DO7clAtoarRNae3MUlekDOyQb8mvK23vwIhBfv453U+n0VhcFWCOyM U4ZtrmACEb3XowJKNsZEKpqQoV90vZgULJXY56Xqf3B6tIXxbxvRqKBJoGNJVmCBGA4A Ks9XraJ/ao/oTOBiac/tmmDk+TW2EllVFkpn0rXi2i4IabmbXIDUBJp9naiJ0uWl+Wy9 XHm7MuxuR2COnADFyDWBvYvbGtk12LhHGOU+vgGV90YdnppJH35ZPBfXaVF18VCOVzlb f5kw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=Im4T5nLORSBFzIzLAcJIJ7W14tpegoLQvSTH9D23aeU=; b=KodaqQqBnmMd8YE+9THBvNeyoTGuxZMBXY2RIGsvK032WOiUORVQ/CvzgRhFvFmjdd bvZbr2+TaqZAn/CNN96Jhm84LyPpK3e+ufAvJozGSFgZ9w3FtDvuAUOJJ3bNU043D333 Ohamdt0M5WaZGdkZsTvG87Q3+8UfBPCw9cyUK1zudHZ6StsB1ViCgmI2igElLiuEjhlC qUL6JJvDkYe7Q2Cx3yZ7LLBoDe7XtfZvWkpwlTnI+QBXLsmsL6cnUalQLO63H8mAa24V UwF5gvUldj1qhNX4Ul5IYnAR2GchYY7Jjg4g8AKmEqWc7zDWyk+uCNlqFshyi8IxCl1q pyWA== X-Gm-Message-State: AOAM530fTApDL+G4sx0sNvtADB7Vk4mq8Cobz7lE0BNpSZaMUWjGurpc +s5p5hl3IY2oohU+j3Mvkgtg5D0SlqnD0g== X-Google-Smtp-Source: ABdhPJygfebM+4hG8VgTKRr7Zc3dy8GBxvc/JS1+KNrzR4mAnzefOENRVMtdpI1RjLS1m3A/XbAnkA== X-Received: by 2002:a05:600c:3d99:b0:39c:55ba:ecc3 with SMTP id bi25-20020a05600c3d9900b0039c55baecc3mr22386212wmb.42.1655484013110; Fri, 17 Jun 2022 09:40:13 -0700 (PDT) Received: from linaro.org ([2a00:23c5:6809:2201:546d:7d59:1703:bf96]) by smtp.gmail.com with ESMTPSA id o18-20020a05600c511200b0039c55bc2c97sm9999761wms.16.2022.06.17.09.40.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Jun 2022 09:40:12 -0700 (PDT) From: Mike Leach To: linux-arm-kernel@lists.infradead.org, coresight@lists.linaro.org Cc: mathieu.poirier@linaro.org, suzuki.poulose@arm.com, leo.yan@linaro.org, Mike Leach Subject: [PATCH v3 2/2] coresight: syscfg: Update load and unload operations Date: Fri, 17 Jun 2022 17:40:08 +0100 Message-Id: <20220617164008.7789-3-mike.leach@linaro.org> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20220617164008.7789-1-mike.leach@linaro.org> References: <20220617164008.7789-1-mike.leach@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220617_094017_580616_497A2DB2 X-CRM114-Status: GOOD ( 32.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org The configfs system is a source of access to the config information in the configuration and feature lists. This can result in additional LOCKDEP issues as a result of the mutex ordering between the config list mutex (cscfg_mutex) and the configfs system mutexes. As such we need to adjust how load/unload operations work to ensure correct operation. 1) Previously the cscfg_mutex was held throughout the load/unload operation. This is now only held during configuration list manipulations, resulting in a multi-stage load/unload process. 2) All operations that manipulate the configfs representation of the configurations and features are now separated out and run without the cscfg_mutex being held. This avoids circular lock_dep issue with the built-in configfs mutexes and semaphores 3) As the load and unload is now multi-stage, some parts under the cscfg_mutex and others not: i) A flag indicating a load / unload operation in progress is used to serialise load / unload operations. ii) activating any configuration not possible when unload is in progress. iii) Configurations have an "available" flag set only after the last load stage for the configuration is complete. Activation of the configuration not possible till flag is set. 4) Following load/unload rules remain: i) Unload prevented while any configuration is active remains. ii) Unload in strict reverse order of load. iii) Existing configurations can be activated while a new load operation is underway. (by definition there can be no dependencies between an existing configuration and a new loading one due to ii) above.) Signed-off-by: Mike Leach --- .../hwtracing/coresight/coresight-config.h | 2 + .../hwtracing/coresight/coresight-syscfg.c | 191 ++++++++++++++---- .../hwtracing/coresight/coresight-syscfg.h | 13 ++ 3 files changed, 168 insertions(+), 38 deletions(-) diff --git a/drivers/hwtracing/coresight/coresight-config.h b/drivers/hwtracing/coresight/coresight-config.h index 2e1670523461..6ba013975741 100644 --- a/drivers/hwtracing/coresight/coresight-config.h +++ b/drivers/hwtracing/coresight/coresight-config.h @@ -134,6 +134,7 @@ struct cscfg_feature_desc { * @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. + * @available: config can be activated - multi-stage load sets true on completion. */ struct cscfg_config_desc { const char *name; @@ -148,6 +149,7 @@ struct cscfg_config_desc { atomic_t active_cnt; void *load_owner; struct config_group *fs_group; + bool available; }; /** diff --git a/drivers/hwtracing/coresight/coresight-syscfg.c b/drivers/hwtracing/coresight/coresight-syscfg.c index 9cd7d3c91d8e..c2364098cabb 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.c +++ b/drivers/hwtracing/coresight/coresight-syscfg.c @@ -460,7 +460,6 @@ static void cscfg_unload_owned_cfgs_feats(void *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); } @@ -469,49 +468,28 @@ static void cscfg_unload_owned_cfgs_feats(void *load_owner) /* 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. - * - * Take a 0 terminated array of feature descriptors and/or configuration - * 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. +/* + * load the features and configs to the lists - called with list mutex held */ -int cscfg_load_config_sets(struct cscfg_config_desc **config_descs, - struct cscfg_feature_desc **feat_descs, - struct cscfg_load_owner_info *owner_info) +static int cscfg_load_owned_cfgs_feats(struct cscfg_config_desc **config_descs, + struct cscfg_feature_desc **feat_descs, + struct cscfg_load_owner_info *owner_info) { - int err = 0, i = 0; - - mutex_lock(&cscfg_mutex); + int i = 0, err = 0; /* load features first */ if (feat_descs) { while (feat_descs[i]) { err = cscfg_load_feat(feat_descs[i]); - if (!err) - err = cscfg_configfs_add_feature(feat_descs[i]); 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; + goto exit_err; } feat_descs[i]->load_owner = owner_info; i++; @@ -523,31 +501,143 @@ int cscfg_load_config_sets(struct cscfg_config_desc **config_descs, if (config_descs) { while (config_descs[i]) { err = cscfg_load_config(config_descs[i]); - if (!err) - err = cscfg_configfs_add_config(config_descs[i]); 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; + goto exit_err; } config_descs[i]->load_owner = owner_info; + config_descs[i]->available = false; + i++; + } + } +exit_err: + return err; +} + +/* set configurations as available to activate at the end of the load process */ +static void cscfg_set_configs_available(struct cscfg_config_desc **config_descs) +{ + int i = 0; + + if (config_descs) { + while (config_descs[i]) { + config_descs[i]->available = true; i++; } } +} + +/* + * Create and register each of the configurations and features with configfs. + * Called without mutex being held. + */ +static int cscfg_fs_register_cfgs_feats(struct cscfg_config_desc **config_descs, + struct cscfg_feature_desc **feat_descs) +{ + int i = 0, err = 0; + + if (feat_descs) { + while (feat_descs[i]) { + err = cscfg_configfs_add_feature(feat_descs[i]); + if (err) + goto exit_err; + i++; + } + } + i = 0; + if (config_descs) { + while (config_descs[i]) { + err = cscfg_configfs_add_config(config_descs[i]); + if (err) + goto exit_err; + i++; + } + } +exit_err: + return err; +} + +/** + * cscfg_load_config_sets - API function to load feature and config sets. + * + * Take a 0 terminated array of feature descriptors and/or configuration + * 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_load_owner_info *owner_info) +{ + int err = 0; + + mutex_lock(&cscfg_mutex); + if (cscfg_mgr->load_op != CSCFG_LOAD_OP_NONE) { + mutex_unlock(&cscfg_mutex); + return -EBUSY; + } + cscfg_mgr->load_op = CSCFG_LOAD_OP_LOAD; + + /* first load and add to the lists */ + err = cscfg_load_owned_cfgs_feats(config_descs, feat_descs, owner_info); + if (err) + goto err_clean_load; /* add the load owner to the load order list */ list_add_tail(&owner_info->item, &cscfg_mgr->load_order_list); if (!list_is_singular(&cscfg_mgr->load_order_list)) { /* lock previous item in load order list */ err = cscfg_owner_get(list_prev_entry(owner_info, item)); - if (err) { - cscfg_unload_owned_cfgs_feats(owner_info); - list_del(&owner_info->item); - } + if (err) + goto err_clean_owner_list; } + /* + * make visible to configfs - configfs manipulation must occur outside + * the list mutex lock to avoid circular lockdep issues with configfs + * built in mutexes and semaphores. This is safe as it is not possible + * to start a new load/unload operation till the current one is done. + */ + mutex_unlock(&cscfg_mutex); + + /* create the configfs elements */ + err = cscfg_fs_register_cfgs_feats(config_descs, feat_descs); + mutex_lock(&cscfg_mutex); + + if (err) + goto err_clean_cfs; + + /* mark any new configs as available for activation */ + cscfg_set_configs_available(config_descs); + goto exit_unlock; + + +err_clean_cfs: + /* cleanup after error registering with configfs */ + cscfg_fs_unregister_cfgs_feats(owner_info); + + if (!list_is_singular(&cscfg_mgr->load_order_list)) + cscfg_owner_put(list_prev_entry(owner_info, item)); + +err_clean_owner_list: + list_del(&owner_info->item); + +err_clean_load: + cscfg_unload_owned_cfgs_feats(owner_info); + exit_unlock: + cscfg_mgr->load_op = CSCFG_LOAD_OP_NONE; mutex_unlock(&cscfg_mutex); return err; } @@ -564,6 +654,9 @@ EXPORT_SYMBOL_GPL(cscfg_load_config_sets); * 1) no configurations are active. * 2) the set being unloaded was the last to be loaded to maintain dependencies. * + * Once the unload operation commences, we disallow any configuration being + * made active until it is complete. + * * @owner_info: Information on owner for set being unloaded. */ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info) @@ -572,6 +665,13 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info) struct cscfg_load_owner_info *load_list_item = NULL; mutex_lock(&cscfg_mutex); + if (cscfg_mgr->load_op != CSCFG_LOAD_OP_NONE) { + mutex_unlock(&cscfg_mutex); + return -EBUSY; + } + + /* unload op in progress also prevents activation of any config */ + cscfg_mgr->load_op = CSCFG_LOAD_OP_UNLOAD; /* cannot unload if anything is active */ if (atomic_read(&cscfg_mgr->sys_active_cnt)) { @@ -592,7 +692,12 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info) goto exit_unlock; } - /* unload all belonging to load_owner */ + /* remove from configfs - again outside the scope of the list mutex */ + mutex_unlock(&cscfg_mutex); + cscfg_fs_unregister_cfgs_feats(owner_info); + mutex_lock(&cscfg_mutex); + + /* unload everything from lists belonging to load_owner */ cscfg_unload_owned_cfgs_feats(owner_info); /* remove from load order list */ @@ -603,7 +708,9 @@ int cscfg_unload_config_sets(struct cscfg_load_owner_info *owner_info) list_del(&owner_info->item); exit_unlock: + cscfg_mgr->load_op = CSCFG_LOAD_OP_NONE; mutex_unlock(&cscfg_mutex); + return err; } EXPORT_SYMBOL_GPL(cscfg_unload_config_sets); @@ -780,8 +887,15 @@ static int _cscfg_activate_config(unsigned long cfg_hash) struct cscfg_config_desc *config_desc; int err = -EINVAL; + if (cscfg_mgr->load_op == CSCFG_LOAD_OP_UNLOAD) + return -EBUSY; + list_for_each_entry(config_desc, &cscfg_mgr->config_desc_list, item) { if ((unsigned long)config_desc->event_ea->var == cfg_hash) { + /* if we happen upon a partly loaded config, can't use it */ + if (config_desc->available == false) + return -EBUSY; + /* must ensure that config cannot be unloaded in use */ err = cscfg_owner_get(config_desc->load_owner); if (err) @@ -1072,6 +1186,7 @@ static int cscfg_create_device(void) INIT_LIST_HEAD(&cscfg_mgr->config_desc_list); INIT_LIST_HEAD(&cscfg_mgr->load_order_list); atomic_set(&cscfg_mgr->sys_active_cnt, 0); + cscfg_mgr->load_op = CSCFG_LOAD_OP_NONE; /* setup the device */ dev = cscfg_device(); diff --git a/drivers/hwtracing/coresight/coresight-syscfg.h b/drivers/hwtracing/coresight/coresight-syscfg.h index 9106ffab4833..e8e812ce7a8b 100644 --- a/drivers/hwtracing/coresight/coresight-syscfg.h +++ b/drivers/hwtracing/coresight/coresight-syscfg.h @@ -12,6 +12,17 @@ #include "coresight-config.h" +/* + * Load operation types. + * When loading or unloading, another load operation cannot be run. + * When unloading configurations cannot be activated. + */ +enum cscfg_load_ops { + CSCFG_LOAD_OP_NONE, + CSCFG_LOAD_OP_LOAD, + CSCFG_LOAD_OP_UNLOAD +}; + /** * System configuration manager device. * @@ -30,6 +41,7 @@ * @cfgfs_subsys: configfs subsystem used to manage configurations. * @sysfs_active_config:Active config hash used if CoreSight controlled from sysfs. * @sysfs_active_preset:Active preset index used if CoreSight controlled from sysfs. + * @load_op: A multi-stage load/unload operation is in progress. */ struct cscfg_manager { struct device dev; @@ -41,6 +53,7 @@ struct cscfg_manager { struct configfs_subsystem cfgfs_subsys; u32 sysfs_active_config; int sysfs_active_preset; + enum cscfg_load_ops load_op; }; /* get reference to dev in cscfg_manager */