From dd026f851195b3cb9773ba6268c99cdf02268725 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Wed, 19 Apr 2017 03:44:20 +0000 Subject: [PATCH 1/8] fileio_tgt: change "#if DEBUG_TM_FN_IGNORE" to "#ifdef ..." Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7137 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- usr/fileio/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/usr/fileio/common.c b/usr/fileio/common.c index d40caafbf..636b0a707 100644 --- a/usr/fileio/common.c +++ b/usr/fileio/common.c @@ -747,7 +747,7 @@ static int process_cmd(struct vdisk_cmd *vcmd) case SCST_USER_TASK_MGMT_DONE: res = do_tm(vcmd, 1); -#if DEBUG_TM_FN_IGNORE +#ifdef DEBUG_TM_FN_IGNORE if (dev->debug_tm_ignore) { sleep(15); } From a8336afa4c1f61552ec95b5302fdb0b20f615355 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 00:57:10 +0000 Subject: [PATCH 2/8] iscsi-scst: change local names of ioctl() and open() to not conflict Change local names of ioctl() and open() to not conflict with C library names. This isn't really a bug in a strictly kernel-resident build, but the change avoids symbol conflicts with libc when building for usermode. Signed-off-by: David Butterfield with some changes git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7138 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/config.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/iscsi-scst/kernel/config.c b/iscsi-scst/kernel/config.c index b4c7e842e..3c2d9f425 100644 --- a/iscsi-scst/kernel/config.c +++ b/iscsi-scst/kernel/config.c @@ -1095,7 +1095,7 @@ out: return res; } -static long ioctl(struct file *file, unsigned int cmd, unsigned long arg) +static long iscsi_ioctl(struct file *file, unsigned int cmd, unsigned long arg) { long err; @@ -1172,7 +1172,7 @@ out: return err; } -static int open(struct inode *inode, struct file *file) +static int iscsi_open(struct inode *inode, struct file *file) { bool already; @@ -1190,7 +1190,7 @@ static int open(struct inode *inode, struct file *file) return 0; } -static int release(struct inode *inode, struct file *filp) +static int iscsi_release(struct inode *inode, struct file *filp) { #ifndef CONFIG_SCST_PROC struct iscsi_attr *attr, *t; @@ -1222,10 +1222,10 @@ static int release(struct inode *inode, struct file *filp) const struct file_operations ctr_fops = { .owner = THIS_MODULE, - .unlocked_ioctl = ioctl, - .compat_ioctl = ioctl, - .open = open, - .release = release, + .unlocked_ioctl = iscsi_ioctl, + .compat_ioctl = iscsi_ioctl, + .open = iscsi_open, + .release = iscsi_release, }; #ifdef CONFIG_SCST_DEBUG From 344387c40e0de2e0fb24c32ffd790bb2b3af4428 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:02:25 +0000 Subject: [PATCH 3/8] iscsi-scstd: replace signal() with sigaction() Replace signal() with sigaction() for validity in a multithreaded process Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7139 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/usr/iscsi_scstd.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/iscsi-scst/usr/iscsi_scstd.c b/iscsi-scst/usr/iscsi_scstd.c index 7fa813d04..d66943087 100644 --- a/iscsi-scst/usr/iscsi_scstd.c +++ b/iscsi-scst/usr/iscsi_scstd.c @@ -846,8 +846,13 @@ int main(int argc, char **argv) /* * Otherwise we could die in some later write() during the event_loop() * instead of getting EPIPE! + * + * The effects of signal(2) in a multithreaded process are unspecified, + * so use sigaction(2) instead. */ - signal(SIGPIPE, SIG_IGN); + struct sigaction act = (struct sigaction) { .sa_handler = SIG_IGN }; + int rc = sigaction(SIGPIPE, &act, NULL); + assert(rc == 0); while ((ch = getopt_long(argc, argv, "c:fd:s:u:g:a:p:vh", long_options, &longindex)) >= 0) { switch (ch) { From 82f5ffd25549c8bdbd4d07da16a343f816c6bacd Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:04:16 +0000 Subject: [PATCH 4/8] iscsi-scstd: daemon handle EOF (rc == 0) from nl_fd Daemon now handles receipt of EOF (rc == 0) from nl_fd. Probably this never happens in a real kernel build, but it does during shutdown in a usermode build and it seems like it is "generically correct" in either case. Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7140 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/usr/event.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/iscsi-scst/usr/event.c b/iscsi-scst/usr/event.c index 8d0734767..1d28ef9e9 100644 --- a/iscsi-scst/usr/event.c +++ b/iscsi-scst/usr/event.c @@ -1066,6 +1066,15 @@ retry: goto retry; log_error("read netlink fd (%d) failed: %s", fd, strerror(errno)); exit(1); + } else if (rc == 0) { + /* + * EOF on nl_fd -- + * We arrive here after the kernel module closes the other end + * of nl_fd during shutdown of the kernel modules. The daemon + * thread is expected to exit when this happens. + */ + log_info("kernel module shutdown -- daemon exits"); + exit(1); } log_debug(1, "target %u, session %#" PRIx64 ", conn %u, code %u, cookie %d", From 660fb57d3a7a9f07e3b9509d1ab0cc4b6787a60a Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:11:37 +0000 Subject: [PATCH 5/8] scst: set file size for NULLIO in PROCFS build The file size wasn't getting set for NULLIO with /proc support Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7141 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/dev_handlers/scst_vdisk.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index 41d93781e..e4ac44897 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -1700,6 +1700,11 @@ next: dev->dev_rd_only = virt_dev->rd_only; +#ifdef CONFIG_SCST_PROC + if (virt_dev->nullio && !virt_dev->file_size) + virt_dev->file_size = VDISK_NULLIO_SIZE; +#endif + res = vdisk_reexamine(virt_dev); if (res < 0) goto out; From 0e92843d30871b76a0503e58f4d793620e6c17ab Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:17:53 +0000 Subject: [PATCH 6/8] iscsi-scst: reset conn->rx_task in scsi_cmnd_start() for EXTRACHECKS In scsi_cmnd_start() conn->rx_task is intended to be set only during the call to _stage1_done, but there was no code to reset it. It doesn't actually matter in execution, but it was a little confusing to wonder and try to understand why it wasn't reset. Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7142 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/iscsi.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/iscsi-scst/kernel/iscsi.c b/iscsi-scst/kernel/iscsi.c index 1a5ac0991..e8dfa6457 100644 --- a/iscsi-scst/kernel/iscsi.c +++ b/iscsi-scst/kernel/iscsi.c @@ -2154,6 +2154,9 @@ static int scsi_cmnd_start(struct iscsi_cmnd *req) req->scst_state = ISCSI_CMD_STATE_RX_CMD; conn->rx_task = current; scst_cmd_init_stage1_done(scst_cmd, SCST_CONTEXT_DIRECT, 0); +#ifdef CONFIG_SCST_EXTRACHECKS + conn->rx_task = NULL; +#endif if (req->scst_state != ISCSI_CMD_STATE_RX_CMD) res = req->conn->transport->iscsit_receive_cmnd_data(req); From 296f133caf784ef4223601aa9e7608162418ac44 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:32:07 +0000 Subject: [PATCH 7/8] iscsi-scst: fix ENOMEM path In an error path in iscsi_threads_pool_get(), when a new pool cannot be allocated, if there is a pool on iscsi_thread_pools_list, it passes that back as an alternative, so return zero in that case. Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7143 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/iscsi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iscsi-scst/kernel/iscsi.c b/iscsi-scst/kernel/iscsi.c index e8dfa6457..54205077f 100644 --- a/iscsi-scst/kernel/iscsi.c +++ b/iscsi-scst/kernel/iscsi.c @@ -4240,7 +4240,7 @@ create: if (p == NULL) { PRINT_ERROR("Unable to allocate iSCSI thread pool (size %zd)", sizeof(*p)); - res = -ENOMEM; + res = 0; if (!list_empty(&iscsi_thread_pools_list)) { PRINT_WARNING("%s", "Using global iSCSI thread pool " "instead"); From a8b24ae319b106c2fb9d671a712627334f368ba0 Mon Sep 17 00:00:00 2001 From: Vladislav Bolkhovitin Date: Fri, 21 Apr 2017 01:36:34 +0000 Subject: [PATCH 8/8] iscsi_scst: conn_lookup() skips any conn already closing This change helped a secondary problem I had under valgrind (due to some other bug) with initiators timing out and reconnecting their sessions faster than the threads were getting cleaned up (for one thing, valgrind only ever runs one thread at a time). It seems logical that the code in conn_lookup() would want to skip any connection that's already known closing in this case, for the same reason it searches the list in reverse. And the dropping connections don't have to drop in order, so searching in reverse doesn't seem sufficient to avoid finding a wrong (stale, closing) connection structure. Signed-off-by: David Butterfield git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7144 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- iscsi-scst/kernel/conn.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/iscsi-scst/kernel/conn.c b/iscsi-scst/kernel/conn.c index e713a1e43..572d6c440 100644 --- a/iscsi-scst/kernel/conn.c +++ b/iscsi-scst/kernel/conn.c @@ -397,7 +397,7 @@ struct iscsi_conn *conn_lookup(struct iscsi_session *session, u16 cid) */ list_for_each_entry_reverse(conn, &session->conn_list, conn_list_entry) { - if (conn->cid == cid) + if (conn->cid == cid && !conn->closing) return conn; } return NULL;