From f955ce890cfd6073aa5aa5a2a10bacd2a0c5cf9f Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 14:42:12 +0100 Subject: [PATCH 1/6] Remove inactive core handling from "remove core" FLush only active core during core removal. During core removal with `casadm -R` there's a flush triggered. This flush shall be skipped for inactive cores. Change return code when `casadm -R` is called with `force` flag. There was no info about dirty data when core was removed without flush. Do not destroy exported object while core is inactive. Perform detach only on active cores. Skip removing inactive core with command for active cores. Signed-off-by: Slawomir Jankowski --- modules/cas_cache/layer_cache_management.c | 48 ++++++++++------------ 1 file changed, 21 insertions(+), 27 deletions(-) diff --git a/modules/cas_cache/layer_cache_management.c b/modules/cas_cache/layer_cache_management.c index 62c70f1..ce814a0 100644 --- a/modules/cas_cache/layer_cache_management.c +++ b/modules/cas_cache/layer_cache_management.c @@ -1363,27 +1363,19 @@ static int _cache_mngt_remove_core_flush(ocf_cache_t cache, core_active = (ocf_core_get_state(core) == ocf_core_state_active); - if (cmd->detach && !core_active) { - printk(KERN_WARNING OCF_PREFIX_SHORT - "Cannot detach core which " - "is already inactive!\n"); - return -OCF_ERR_CORE_IN_INACTIVE_STATE; - } - - if (core_active) { - return _cache_mngt_core_flush_sync(core, - true, _cache_read_unlock_put_cmpl); - } else if (!ocf_mngt_core_is_dirty(core)) { - result = 0; - goto unlock; - } else { - printk(KERN_WARNING OCF_PREFIX_SHORT - "Cannot remove dirty inactive core " - "without force option\n"); + if (!core_active) { result = -OCF_ERR_CORE_IN_INACTIVE_STATE; goto unlock; } + if (!ocf_mngt_core_is_dirty(core)) { + result = 0; + goto unlock; + } + + return _cache_mngt_core_flush_sync(core, true, + _cache_read_unlock_put_cmpl); + unlock: ocf_mngt_cache_read_unlock(cache); put: @@ -1399,21 +1391,23 @@ static int _cache_mngt_remove_core_prepare(ocf_cache_t cache, ocf_core_t core, core_active = ocf_core_get_state(core) == ocf_core_state_active; - if (cmd->detach && !core_active) { - printk(KERN_WARNING OCF_PREFIX_SHORT - "Cannot detach core which " - "is already inactive!\n"); + if (!core_active) { + if (cmd->detach) { + printk(KERN_WARNING OCF_PREFIX_SHORT + "Cannot detach core which " + "is already inactive!\n"); + } return -OCF_ERR_CORE_IN_INACTIVE_STATE; - } - - if (core_active) { + } else { result = block_dev_destroy_exported_object(core); if (result) return result; } - if (!cmd->force_no_flush) - result = _cache_mngt_core_flush_uninterruptible(core); + if (cmd->force_no_flush) + return -KCAS_ERR_REMOVED_DIRTY; + + result = _cache_mngt_core_flush_uninterruptible(core); return result ? -KCAS_ERR_REMOVED_DIRTY : 0; } @@ -1463,7 +1457,7 @@ int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd) init_completion(&context.cmpl); context.result = &result; - if (cmd->detach || prepare_result == -KCAS_ERR_REMOVED_DIRTY) { + if (cmd->detach) { ocf_mngt_cache_detach_core(core, _cache_mngt_remove_core_complete, &context); } else { From 2bf6e42dea7264a30bebb4e0a98f4367d313f229 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 15:35:57 +0100 Subject: [PATCH 2/6] Print separate messages for different "remove core" return codes Change extended error message for `KCAS_ERR_REMOVED_DIRTY`. Print informative messages when `remove core` command fails. Make separate error messages for detaching. Update help printouts. Update documentation comments. Signed-off-by: Slawomir Jankowski --- casadm/cas_lib.c | 20 ++++++++++++++++---- casadm/cas_main.c | 4 ++-- casadm/extended_err_msg.c | 4 ++-- modules/include/cas_ioctl_codes.h | 8 ++++---- 4 files changed, 24 insertions(+), 12 deletions(-) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 6d8d397..043fedc 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -1904,12 +1904,24 @@ int remove_core(unsigned int cache_id, unsigned int core_id, if (run_ioctl_interruptible(fd, KCAS_IOCTL_REMOVE_CORE, &cmd, "Removing core", cache_id, core_id) < 0) { close(fd); - if (OCF_ERR_FLUSHING_INTERRUPTED == cmd.ext_err_code) { - cas_printf(LOG_ERR, "You have interrupted removal of core. CAS continues to operate normally.\n"); + if (cmd.ext_err_code == OCF_ERR_FLUSHING_INTERRUPTED) { + cas_printf(LOG_ERR, "You have interrupted %s of core. " + "CAS continues to operate normally.\n", + detach ? "detaching" : "removal"); return INTERRUPTED; + } else if (cmd.ext_err_code == OCF_ERR_CORE_IN_INACTIVE_STATE) { + cas_printf(LOG_ERR, "Core is inactive. To manage the " + "inactive core use '--remove-inactive' " + "command.\n"); + return FAILURE; + } else if (cmd.ext_err_code == KCAS_ERR_REMOVED_DIRTY) { + print_err(cmd.ext_err_code); + return SUCCESS; } else { - cas_printf(LOG_ERR, "Error while removing core device %d from cache instance %d\n", - core_id, cache_id); + cas_printf(LOG_ERR, "Error while %s core device %d " + "from cache instance %d\n", + detach ? "detaching" : "removing", + core_id, cache_id); print_err(cmd.ext_err_code); return FAILURE; } diff --git a/casadm/cas_main.c b/casadm/cas_main.c index bba3b58..6093906 100644 --- a/casadm/cas_main.c +++ b/casadm/cas_main.c @@ -1109,7 +1109,7 @@ int handle_add() static cli_option remove_options[] = { {'i', "cache-id", CACHE_ID_DESC, 1, "ID", CLI_OPTION_REQUIRED}, {'j', "core-id", CORE_ID_DESC, 1, "ID", CLI_OPTION_REQUIRED}, - {'f', "force", "Force remove inactive core"}, + {'f', "force", "Force active core removal without data flush"}, {0} }; @@ -1961,7 +1961,7 @@ static cli_command cas_commands[] = { { .name = "remove-core", .short_name = 'R', - .desc = "Remove core device from cache instance", + .desc = "Remove active core device from cache instance", .long_desc = NULL, .options = remove_options, .command_handle_opts = remove_core_command_handle_option, diff --git a/casadm/extended_err_msg.c b/casadm/extended_err_msg.c index e70ea2c..6b4b842 100644 --- a/casadm/extended_err_msg.c +++ b/casadm/extended_err_msg.c @@ -222,8 +222,8 @@ struct { }, { KCAS_ERR_REMOVED_DIRTY, - "Flush error occured. Core has been set to detached state.\n" - "Warning: Core device may contain inconsistent data.\n" + "Warning: Core has been removed or detached without flush.\n" + "Core device may contain inconsistent data.\n" "To access your data please add core back to the cache." }, { diff --git a/modules/include/cas_ioctl_codes.h b/modules/include/cas_ioctl_codes.h index 32520fe..ff5c582 100644 --- a/modules/include/cas_ioctl_codes.h +++ b/modules/include/cas_ioctl_codes.h @@ -117,7 +117,7 @@ struct kcas_insert_core { struct kcas_remove_core { uint16_t cache_id; /**< id of an running cache */ uint16_t core_id; /**< id core object to be removed */ - bool force_no_flush; /**< remove core without flushing */ + bool force_no_flush; /**< remove active core without flushing */ bool detach; /**< detach core without removing it from cache metadata */ int ext_err_code; @@ -473,7 +473,7 @@ struct kcas_get_cache_param { /** Add core object to an running cache instance */ #define KCAS_IOCTL_INSERT_CORE _IOWR(KCAS_IOCTL_MAGIC, 22, struct kcas_insert_core) -/** Remove core object from an running cache instance */ +/** Remove active core object from an running cache instance */ #define KCAS_IOCTL_REMOVE_CORE _IOR(KCAS_IOCTL_MAGIC, 23, struct kcas_remove_core) /** Retrieve properties of a running cache instance (incl. mode etc.) */ @@ -566,7 +566,7 @@ enum kcas_error { /** Given device is a partition */ KCAS_ERR_A_PART, - /** Core has been removed with flush error */ + /** Core has been removed, but it may contain dirty data */ KCAS_ERR_REMOVED_DIRTY, /** Cache has been stopped, but it may contain dirty data */ @@ -584,7 +584,7 @@ enum kcas_error { /** Condition token does not identify any known condition */ KCAS_ERR_CLS_RULE_UNKNOWN_CONDITION, - /** Waiting for async operation was interrupted*/ + /** Waiting for async operation was interrupted */ KCAS_ERR_WAITING_INTERRUPTED, /** Cache already being stopped*/ From 760b60628c142e353015cde6c75f00a4886354c8 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 15:48:15 +0100 Subject: [PATCH 3/6] Add `KCAS_ERR_CORE_IN_ACTIVE_STATE` error code. New error code will allow to properly handle issues caused by wrong usage of `remove inactive core` command. It will also allow to print meaningful error messages. Signed-off-by: Slawomir Jankowski --- casadm/extended_err_msg.c | 4 ++++ modules/cas_cache/utils/cas_err.h | 1 + modules/include/cas_ioctl_codes.h | 3 +++ 3 files changed, 8 insertions(+) diff --git a/casadm/extended_err_msg.c b/casadm/extended_err_msg.c index 6b4b842..dd8b1c9 100644 --- a/casadm/extended_err_msg.c +++ b/casadm/extended_err_msg.c @@ -256,7 +256,11 @@ struct { KCAS_ERR_CACHE_STOPPING, "Cache being stopped" }, + { + KCAS_ERR_CORE_IN_ACTIVE_STATE, + "Core device is in active state" + }, }; diff --git a/modules/cas_cache/utils/cas_err.h b/modules/cas_cache/utils/cas_err.h index fd4badc..c0443ac 100644 --- a/modules/cas_cache/utils/cas_err.h +++ b/modules/cas_cache/utils/cas_err.h @@ -65,6 +65,7 @@ struct { { KCAS_ERR_A_PART, EINVAL }, { KCAS_ERR_REMOVED_DIRTY, EIO }, { KCAS_ERR_STOPPED_DIRTY, EIO }, + { KCAS_ERR_CORE_IN_ACTIVE_STATE, ENODEV }, }; /*******************************************/ diff --git a/modules/include/cas_ioctl_codes.h b/modules/include/cas_ioctl_codes.h index ff5c582..4b98ceb 100644 --- a/modules/include/cas_ioctl_codes.h +++ b/modules/include/cas_ioctl_codes.h @@ -589,6 +589,9 @@ enum kcas_error { /** Cache already being stopped*/ KCAS_ERR_CACHE_STOPPING, + + /** Core device is in active state */ + KCAS_ERR_CORE_IN_ACTIVE_STATE }; #endif From 2514f5fa5bac260772f273193599d80a3c0f40a2 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 15:59:11 +0100 Subject: [PATCH 4/6] Add "remove inactive" command to casadm Add `remove_inactive_core` function to casadm's code to handle `remove inactive` command. Print messages based on errors returned when command fails. Add documentation comment to new function. Add CLI part of introduced command. Signed-off-by: Slawomir Jankowski --- casadm/cas_lib.c | 37 +++++++++++++++++++++++++++++++++++++ casadm/cas_lib.h | 9 +++++++++ casadm/cas_main.c | 39 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 85 insertions(+) diff --git a/casadm/cas_lib.c b/casadm/cas_lib.c index 043fedc..88dbbeb 100644 --- a/casadm/cas_lib.c +++ b/casadm/cas_lib.c @@ -1931,6 +1931,43 @@ int remove_core(unsigned int cache_id, unsigned int core_id, return SUCCESS; } +int remove_inactive_core(unsigned int cache_id, unsigned int core_id) +{ + int fd = 0; + struct kcas_remove_inactive cmd; + + /* don't even attempt ioctl if filesystem is mounted */ + if (SUCCESS != check_if_mounted(cache_id, core_id)) { + return FAILURE; + } + + fd = open_ctrl_device(); + if (fd == -1) + return FAILURE; + + memset(&cmd, 0, sizeof(cmd)); + cmd.cache_id = cache_id; + cmd.core_id = core_id; + + if (run_ioctl(fd, KCAS_IOCTL_REMOVE_INACTIVE, &cmd) < 0) { + close(fd); + if (cmd.ext_err_code == KCAS_ERR_CORE_IN_ACTIVE_STATE) { + cas_printf(LOG_ERR, "Core is active. " + "To manage the active core use " + "'--remove-core' command.\n"); + } else { + cas_printf(LOG_ERR, "Error while removing inactive " + "core device %d from cache instance " + "%d\n", core_id, cache_id); + print_err(cmd.ext_err_code); + } + return FAILURE; + } + close(fd); + + return SUCCESS; +} + int core_pool_remove(const char *core_device) { struct kcas_core_pool_remove cmd; diff --git a/casadm/cas_lib.h b/casadm/cas_lib.h index 5bf3d34..020365e 100644 --- a/casadm/cas_lib.h +++ b/casadm/cas_lib.h @@ -215,6 +215,15 @@ int get_core_info(int fd, int cache_id, int core_id, struct kcas_core_info *info int remove_core(unsigned int cache_id, unsigned int core_id, bool detach, bool force_no_flush); +/** + * @brief remove inactive core device from a cache + * + * @param cache_id cache from which inactive core is being removed + * @param cache_id inactive core which is being removed + * @return 0 upon successful core removal, 1 upon failure + */ +int remove_inactive_core(unsigned int cache_id, unsigned int core_id); + int core_pool_remove(const char *core_device); int get_core_pool_count(int fd); diff --git a/casadm/cas_main.c b/casadm/cas_main.c index 6093906..208ec3d 100644 --- a/casadm/cas_main.c +++ b/casadm/cas_main.c @@ -180,6 +180,23 @@ int remove_core_command_handle_option(char *opt, const char **arg) return 0; } +int remove_inactive_core_command_handle_option(char *opt, const char **arg) +{ + if (!strcmp(opt, "cache-id")){ + if (validate_str_num(arg[0], "cache id", OCF_CACHE_ID_MIN, OCF_CACHE_ID_MAX) == FAILURE) + return FAILURE; + + command_args_values.cache_id = atoi(arg[0]); + } else if (!strcmp(opt, "core-id")){ + if (validate_str_num(arg[0], "core id", 0, OCF_CORE_ID_MAX) == FAILURE) + return FAILURE; + + command_args_values.core_id = atoi(arg[0]); + } + + return 0; +} + int core_pool_remove_command_handle_option(char *opt, const char **arg) { if (!strcmp(opt, "device")) { @@ -1121,6 +1138,18 @@ int handle_remove() command_args_values.force); } +static cli_option remove_inactive_options[] = { + {'i', "cache-id", CACHE_ID_DESC, 1, "ID", CLI_OPTION_REQUIRED}, + {'j', "core-id", CORE_ID_DESC, 1, "ID", CLI_OPTION_REQUIRED}, + {0} +}; + +int handle_remove_inactive() +{ + return remove_inactive_core(command_args_values.cache_id, + command_args_values.core_id); +} + static cli_option core_pool_remove_options[] = { {'d', "device", CORE_DEVICE_DESC, 1, "DEVICE", CLI_OPTION_REQUIRED}, {0} @@ -1969,6 +1998,16 @@ static cli_command cas_commands[] = { .flags = CLI_SU_REQUIRED, .help = NULL, }, + { + .name = "remove-inactive", + .desc = "Remove inactive core device from cache instance", + .long_desc = NULL, + .options = remove_inactive_options, + .command_handle_opts = remove_inactive_core_command_handle_option, + .handle = handle_remove_inactive, + .flags = CLI_SU_REQUIRED, + .help = NULL, + }, { .name = "remove-detached", .desc = "Remove core device from core pool", From 696a2e175c2badf56cc4f01ea2c12b2a54b5f10d Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 16:02:11 +0100 Subject: [PATCH 5/6] Add `KCAS_IOCTL_REMOVE_INACTIVE` request to cas_cache module Add IOCTL code and struct for above request. Signed-off-by: Slawomir Jankowski --- modules/cas_cache/service_ui_ioctl.c | 10 ++++++++++ modules/include/cas_ioctl_codes.h | 11 +++++++++++ 2 files changed, 21 insertions(+) diff --git a/modules/cas_cache/service_ui_ioctl.c b/modules/cas_cache/service_ui_ioctl.c index f91d240..2fc0246 100644 --- a/modules/cas_cache/service_ui_ioctl.c +++ b/modules/cas_cache/service_ui_ioctl.c @@ -132,6 +132,16 @@ long cas_service_ioctl_ctrl(struct file *filp, unsigned int cmd, RETURN_CMD_RESULT(cmd_info, arg, retval); } + case KCAS_IOCTL_REMOVE_INACTIVE: { + struct kcas_remove_inactive *cmd_info; + + GET_CMD_INFO(cmd_info, arg); + + retval = cache_mngt_remove_inactive_core(cmd_info); + + RETURN_CMD_RESULT(cmd_info, arg, retval); + } + case KCAS_IOCTL_RESET_STATS: { struct kcas_reset_stats *cmd_info; char cache_name[OCF_CACHE_NAME_SIZE]; diff --git a/modules/include/cas_ioctl_codes.h b/modules/include/cas_ioctl_codes.h index 4b98ceb..4ef1358 100644 --- a/modules/include/cas_ioctl_codes.h +++ b/modules/include/cas_ioctl_codes.h @@ -123,6 +123,13 @@ struct kcas_remove_core { int ext_err_code; }; +struct kcas_remove_inactive { + uint16_t cache_id; /**< id of an running cache */ + uint16_t core_id; /**< id core object to be removed */ + + int ext_err_code; +}; + struct kcas_reset_stats { uint16_t cache_id; /**< id of an running cache */ uint16_t core_id; /**< id core object to be removed */ @@ -419,6 +426,7 @@ struct kcas_get_cache_param { * 34 * KCAS_IOCTL_GET_STATS * OK * * 35 * KCAS_IOCTL_PURGE_CACHE * OK * * 36 * KCAS_IOCTL_PURGE_CORE * OK * + * 37 * KCAS_IOCTL_REMOVE_INACTIVE * OK * ******************************************************************************* */ @@ -517,6 +525,9 @@ struct kcas_get_cache_param { * and invalidate all valid cache lines associated with given core. */ #define KCAS_IOCTL_PURGE_CORE _IOWR(KCAS_IOCTL_MAGIC, 36, struct kcas_flush_core) +/** Remove inactive core object from an running cache instance */ +#define KCAS_IOCTL_REMOVE_INACTIVE _IOR(KCAS_IOCTL_MAGIC, 37, struct kcas_remove_inactive) + /** * Extended kernel CAS error codes */ From 461a4f2d96e9fceab0bf995b07333f2170150971 Mon Sep 17 00:00:00 2001 From: Slawomir Jankowski Date: Wed, 17 Feb 2021 16:11:28 +0100 Subject: [PATCH 6/6] Introduce "remove inactive core" to cas_cache module Create module-side handling of inactive core removal. Extract core functionality of core removal that applies to inactive core and copy it to `cache_mngt_remove_inactive_core` function. Return proper error if core is active. Signed-off-by: Slawomir Jankowski --- modules/cas_cache/layer_cache_management.c | 54 ++++++++++++++++++++++ modules/cas_cache/layer_cache_management.h | 2 + 2 files changed, 56 insertions(+) diff --git a/modules/cas_cache/layer_cache_management.c b/modules/cas_cache/layer_cache_management.c index ce814a0..9f0ad9d 100644 --- a/modules/cas_cache/layer_cache_management.c +++ b/modules/cas_cache/layer_cache_management.c @@ -1484,6 +1484,60 @@ put: return result; } +int cache_mngt_remove_inactive_core(struct kcas_remove_inactive *cmd) +{ + struct _cache_mngt_sync_context context; + int result = 0; + ocf_cache_t cache; + ocf_core_t core; + + result = mngt_get_cache_by_id(cas_ctx, cmd->cache_id, &cache); + if (result) + return result; + + result = _cache_mngt_lock_sync(cache); + if (result) + goto put; + + result = get_core_by_id(cache, cmd->core_id, &core); + if (result < 0) { + goto unlock; + } + + result = (ocf_core_get_state(core) == ocf_core_state_active); + if (result) { + result = -KCAS_ERR_CORE_IN_ACTIVE_STATE; + goto unlock; + } + + /* + * Destroy exported object - in case of error during destruction of + * exported object, instead of trying rolling this back we rather + * inform user about error. + */ + result = block_dev_destroy_exported_object(core); + if (result) + goto unlock; + + init_completion(&context.cmpl); + context.result = &result; + + ocf_mngt_cache_remove_core(core, _cache_mngt_remove_core_complete, + &context); + + wait_for_completion(&context.cmpl); + + if (!result) { + mark_core_id_free(cache, cmd->core_id); + } + +unlock: + ocf_mngt_cache_unlock(cache); +put: + ocf_mngt_cache_put(cache); + return result; +} + int cache_mngt_reset_stats(const char *cache_name, size_t cache_name_len, const char *core_name, size_t core_name_len) { diff --git a/modules/cas_cache/layer_cache_management.h b/modules/cas_cache/layer_cache_management.h index 62f984e..398aad5 100644 --- a/modules/cas_cache/layer_cache_management.h +++ b/modules/cas_cache/layer_cache_management.h @@ -36,6 +36,8 @@ int cache_mngt_add_core_to_cache(const char *cache_name, size_t name_len, int cache_mngt_remove_core_from_cache(struct kcas_remove_core *cmd); +int cache_mngt_remove_inactive_core(struct kcas_remove_inactive *cmd); + int cache_mngt_reset_stats(const char *cache_name, size_t cache_name_len, const char *core_name, size_t core_name_len);