From 785e721cf3489cc376f8c05c47bb7d175da9503b Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sun, 23 Feb 2020 23:05:13 +0000 Subject: [PATCH] scst: Add the 'forward_src' sysfs attribute Make forwarding source mode configurable per target port instead of having a compile-time global option. git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@8776 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/ChangeLog | 5 ++++ scst/README | 25 ++++++++++---------- scst/include/backport.h | 14 +++++++++++ scst/include/scst.h | 13 ++++++++++- scst/src/dev_handlers/scst_disk.c | 10 ++++---- scst/src/scst_local_cmd.c | 4 +--- scst/src/scst_main.c | 13 +++-------- scst/src/scst_sysfs.c | 39 +++++++++++++++++++++++++++++++ 8 files changed, 90 insertions(+), 33 deletions(-) diff --git a/scst/ChangeLog b/scst/ChangeLog index 8176681bd..140030676 100644 --- a/scst/ChangeLog +++ b/scst/ChangeLog @@ -1,3 +1,8 @@ +Summary of changes between versions 3.4 and 3.5 +----------------------------------------------- +- Added the forward_src and forward_dst sysfs attributes. Removed + CONFIG_SCST_FORWARD_MODE_PASS_THROUGH. + Summary of changes between versions 3.3 and 3.4 ----------------------------------------------- - I/O keeps running at full speed if a LUN is added or removed and also if diff --git a/scst/README b/scst/README index 6091a9c06..64b7ea8e5 100644 --- a/scst/README +++ b/scst/README @@ -343,16 +343,6 @@ in/out in Makefile and scst.h: functionality is working only if dif_mode doesn't contain dev_store and dif_type is 1. - - CONFIG_SCST_FORWARD_MODE_PASS_THROUGH - if defined, the pass-through - subsystem starts working in the forwarding mode, where reservation - commands processed locally and not passed to the backend SCSI device, - while COMPARE AND WRITE, EXTENDED COPY and RECEIVE COPY RESULTS - commands, which normally processed locally by the SCST core, not - processed locally, but passed to the backend device. Intended to be - used to implement NON-OPTIMIZED ALUA state together with "forwarding" - target attribute on the remote node. See below for more details. - Disabled by default for safety. - - CONFIG_SCST_NO_TOTAL_MEM_CHECKS - disables checks of allocated memory, see scst_max_cmd_mem below. Allows to avoid 2 global variables on the fast path, hence get better multi-queue performance. @@ -807,6 +797,14 @@ Every target should have at least the following entries: until rel_tgt_id becomes unique. This attribute initialized unique by SCST by default. + - forward_src - if set this target port is a forwarding source. This means + that commands like COMPARE AND WRITE, EXTENDED COPY and RECEIVE COPY + RESULTS are submitted to the SCSI device instead of being handled inside + the SCST core. PERSISTENT RESERVE IN and OUT commands are processed by the + SCST core, whether or not this mode is enabled. The name 'forwarding_src' + refers to the use case where SCSI passthrough is used to send SCSI commands + to another H.A. node. + - forward_dst - if set this target port is a forwarding destination. This means that it does not check any local SCSI events (reservations, etc.). Those event are supposed to be checked at the forwarding source side. @@ -1986,13 +1984,13 @@ The link between block devices I and J stands for synchronous replication. Such a setup can be configured as follows: -1. Build SCST with CONFIG_SCST_FORWARD_MODE_PASS_THROUGH enabled in scst.h +1. Build SCST. 2. Setup on active node internal redirect target, which is going to accept redirected commands from the passive node. It must be visible only to the passive node. -3. Set "forwarding" attribute for this target to 1. This is necessary to +3. Set "forward_dst" attribute for this target to 1. This is necessary to correctly handle PRs. 4. Export through this target the SAME backend SCST device as being @@ -2007,7 +2005,8 @@ device on the passive side pointing to the active node. *pass-through* handler (scst_disk). Pass-though is needed to redirect non-block commands as well: ATS, XCOPY, etc. -7. Set ALUA state to this target as "nonoptimized". +7. Set ALUA state to this target as "nonoptimized". Set the forward_src +attribute to one. That's it on the normal path. Now the initiator(s) would see 2 paths: OPTIMIZED going to the active node and NON-OPTIMIZED going to the diff --git a/scst/include/backport.h b/scst/include/backport.h index 32d624d5d..566ad0112 100644 --- a/scst/include/backport.h +++ b/scst/include/backport.h @@ -674,6 +674,20 @@ static inline int __must_check kstrtol(const char *s, unsigned int base, { return strict_strtol(s, base, res); } + +static inline int __must_check kstrtoint(const char *s, unsigned int base, + int *result) +{ + long val; + int ret = strict_strtol(s, base, &val); + + if (ret) + return ret; + *result = val; + if (*result != val) + return -EINVAL; + return 0; +} #endif #if LINUX_VERSION_CODE < KERNEL_VERSION(4, 17, 0) diff --git a/scst/include/scst.h b/scst/include/scst.h index f64caec8d..17939b18a 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -28,7 +28,6 @@ /** See README for description of those conditional defines **/ #define CONFIG_SCST_DIF_INJECT_CORRUPTED_TAGS -/* #define CONFIG_SCST_FORWARD_MODE_PASS_THROUGH */ #define CONFIG_SCST_NO_TOTAL_MEM_CHECKS #include @@ -1687,6 +1686,18 @@ struct scst_tgt { struct list_head tgt_acg_list; /* target ACG groups */ + /* + * Set if and only if this target port is a forwarding source. If + * enabled, the dev_disk handler submits COMPARE AND WRITE, EXTENDED + * COPY and RECEIVE COPY RESULTS commands to the SCSI device instead + * of handling these inside the SCST core. PERSISTENT RESERVE IN and + * OUT commands are processed by the SCST core, whether or not this + * mode is enabled. The name 'forwarding_src' refers to the use case + * where SCSI passthrough is used to send SCSI commands to another + * H.A. node. + */ + unsigned tgt_forward_src:1; + /* * Set, if this target is forwarding destination, i.e. does not check * any local SCSI events (reservations, etc.). Those events are diff --git a/scst/src/dev_handlers/scst_disk.c b/scst/src/dev_handlers/scst_disk.c index cd2ab0bff..785d4bcf7 100644 --- a/scst/src/dev_handlers/scst_disk.c +++ b/scst/src/dev_handlers/scst_disk.c @@ -144,6 +144,7 @@ static void disk_detach(struct scst_device *dev) static int disk_parse(struct scst_cmd *cmd) { + struct scst_tgt *tgt = cmd->tgt; int res = SCST_CMD_STATE_DEFAULT, rc; rc = scst_sbc_generic_parse(cmd); @@ -152,8 +153,7 @@ static int disk_parse(struct scst_cmd *cmd) goto out; } -#ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH - if (unlikely(cmd->op_flags & SCST_LOCAL_CMD)) { + if (unlikely(tgt->tgt_forward_src && cmd->op_flags & SCST_LOCAL_CMD)) { switch (cmd->cdb[0]) { case COMPARE_AND_WRITE: case EXTENDED_COPY: @@ -164,7 +164,6 @@ static int disk_parse(struct scst_cmd *cmd) break; } } -#endif cmd->retries = SCST_PASSTHROUGH_RETRIES; out: @@ -286,6 +285,7 @@ out_complete: /* Executes command and split CDB, if necessary */ static enum scst_exec_res disk_exec(struct scst_cmd *cmd) { + struct scst_tgt *tgt = cmd->tgt; enum scst_exec_res res; int rc; struct disk_work work; @@ -299,8 +299,7 @@ static enum scst_exec_res disk_exec(struct scst_cmd *cmd) TRACE_ENTRY(); -#ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH - if (unlikely(cmd->op_flags & SCST_LOCAL_CMD)) { + if (unlikely(tgt->tgt_forward_src && cmd->op_flags & SCST_LOCAL_CMD)) { switch (cmd->cdb[0]) { case RESERVE: case RESERVE_10: @@ -315,7 +314,6 @@ static enum scst_exec_res disk_exec(struct scst_cmd *cmd) break; } } -#endif /* * For PC requests we are going to submit max_hw_sectors used instead diff --git a/scst/src/scst_local_cmd.c b/scst/src/scst_local_cmd.c index 2abea3b0b..757edba71 100644 --- a/scst/src/scst_local_cmd.c +++ b/scst/src/scst_local_cmd.c @@ -714,15 +714,13 @@ enum scst_exec_res scst_persistent_reserve_in_local(struct scst_cmd *cmd) goto out_done; } -#ifndef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH - if (dev->scsi_dev != NULL) { + if (cmd->tgt->tgt_forward_src && dev->scsi_dev) { PRINT_WARNING("PR commands for pass-through devices not " "supported (device %s)", dev->virt_name); scst_set_cmd_error(cmd, SCST_LOAD_SENSE(scst_sense_invalid_opcode)); goto out_done; } -#endif buffer_size = scst_get_buf_full_sense(cmd, &buffer); if (unlikely(buffer_size <= 0)) diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index f03210d4e..21c1fd517 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -1111,7 +1111,7 @@ static int scst_register_device(struct scsi_device *scsidp) dev->scsi_dev = scsidp; -#ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH + /* For target ports that function as forwarding source. */ res = scst_pr_set_file_name(dev, NULL, "%s/%s", SCST_PR_DIR, dev->virt_name); if (res != 0) @@ -1120,7 +1120,6 @@ static int scst_register_device(struct scsi_device *scsidp) res = scst_pr_init_dev(dev); if (res != 0) goto out_free_dev; -#endif list_add_tail(&dev->dev_list_entry, &scst_dev_list); @@ -1143,14 +1142,9 @@ out: out_del_unlocked: mutex_lock(&scst_mutex); list_del_init(&dev->dev_list_entry); - mutex_unlock(&scst_mutex); - percpu_ref_kill(&dev->refcnt); - goto out; - -#ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH + /* If used as a forwarding source (see also tgt_forward_src). */ scst_pr_clear_dev(dev); -#endif out_free_dev: percpu_ref_kill(&dev->refcnt); @@ -1193,9 +1187,8 @@ static void scst_unregister_device(struct scsi_device *scsidp) list_del_init(&dev->dev_list_entry); -#ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH + /* For forwarding sources (see also tgt_forward_src). */ scst_pr_clear_dev(dev); -#endif scst_dg_dev_remove_by_dev(dev); diff --git a/scst/src/scst_sysfs.c b/scst/src/scst_sysfs.c index f47154620..7381dee96 100644 --- a/scst/src/scst_sysfs.c +++ b/scst/src/scst_sysfs.c @@ -2513,6 +2513,44 @@ static struct kobj_attribute scst_rel_tgt_id = __ATTR(rel_tgt_id, S_IRUGO | S_IWUSR, scst_rel_tgt_id_show, scst_rel_tgt_id_store); +static ssize_t scst_tgt_forward_src_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) +{ + struct scst_tgt *tgt = container_of(kobj, struct scst_tgt, tgt_kobj); + + return sprintf(buf, "%d\n%s", tgt->tgt_forward_src, + tgt->tgt_forward_src ? SCST_SYSFS_KEY_MARK "\n" : ""); +} + +static ssize_t scst_tgt_forward_src_store(struct kobject *kobj, + struct kobj_attribute *attr, const char *buf, size_t count) +{ + struct scst_tgt *tgt = container_of(kobj, struct scst_tgt, tgt_kobj); + int res, old, new; + + res = kstrtoint(buf, 0, &new); + if (res < 0) + return res; + if (new < 0 || new > 1) + return -EINVAL; + + mutex_lock(&scst_mutex); + old = tgt->tgt_forward_src; + if (old != new) { + tgt->tgt_forward_src = new; + PRINT_INFO("%s target %s as forwarding source", + tgt->tgt_forward_src ? "Set" : "Clear", + tgt->tgt_name); + } + mutex_unlock(&scst_mutex); + + return count; +} + +static struct kobj_attribute scst_tgt_forward_src = + __ATTR(forward_src, S_IRUGO | S_IWUSR, scst_tgt_forward_src_show, + scst_tgt_forward_src_store); + static ssize_t scst_tgt_forward_dst_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { @@ -2885,6 +2923,7 @@ SCST_TGT_SYSFS_STAT_ATTR(cmd_count, none_cmd_count, SCST_DATA_NONE, >> 0); static struct attribute *scst_tgt_attrs[] = { &scst_rel_tgt_id.attr, + &scst_tgt_forward_src.attr, &scst_tgt_forward_dst.attr, &scst_tgt_forwarding.attr, &scst_tgt_comment.attr,