From 457d6fceed56f28d5af36fc7aaccbe051d3cefe7 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 5 Jan 2019 21:42:33 +0000 Subject: [PATCH 1/3] scst, scst_vdisk: Simplify the code for querying the vdisk filename git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7860 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/include/scst.h | 8 -------- scst/src/dev_handlers/scst_vdisk.c | 25 ++++++++++++------------- scst/src/scst_main.c | 7 +------ 3 files changed, 13 insertions(+), 27 deletions(-) diff --git a/scst/include/scst.h b/scst/include/scst.h index 1679a492d..7331da662 100644 --- a/scst/include/scst.h +++ b/scst/include/scst.h @@ -2846,14 +2846,6 @@ struct scst_device { /* Set, if a strictly serialized cmd is waiting blocked */ unsigned int strictly_serialized_cmd_waiting:1; - /* - * Set, if this device is being unregistered. Useful to let sysfs - * attributes know when they should exit immediately to prevent - * possible deadlocks with their device unregistration waiting for - * their kobj last put. - */ - unsigned int dev_unregistering:1; - /* * Set if ext blocking is pending. It is just shortcut for * !list_empty(&dev->ext_blockers_list) to save a cache miss. diff --git a/scst/src/dev_handlers/scst_vdisk.c b/scst/src/dev_handlers/scst_vdisk.c index d05f8f456..3931a87c9 100644 --- a/scst/src/dev_handlers/scst_vdisk.c +++ b/scst/src/dev_handlers/scst_vdisk.c @@ -9154,6 +9154,17 @@ static ssize_t vdisk_sysfs_rotational_show(struct kobject *kobj, return pos; } +static bool scst_dev_being_unregistered(struct scst_device *dev) +{ + bool res; + + mutex_lock(&scst_mutex); + res = list_empty(&dev->dev_list_entry); + mutex_unlock(&scst_mutex); + + return res; +} + static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work) { int res = 0; @@ -9171,7 +9182,7 @@ static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work) * under scst_vdisk_mutex. */ while (!mutex_trylock(&scst_vdisk_mutex)) { - if (dev->dev_unregistering) { + if (scst_dev_being_unregistered(dev)) { TRACE_MGMT_DBG("Skipping being unregistered dev %s", dev->virt_name); res = -ENOENT; @@ -9182,18 +9193,6 @@ static int vdev_sysfs_process_get_filename(struct scst_sysfs_work_item *work) goto out_put; } msleep(100); - /* - * We need to reread dev_unregistering from memory, hence - * prevent compiler from putting it in a register. Generally, - * it shouldn't happen, because the compiler isn't allowed to do - * such a transformation if any functions that can cause side - * effects are called between successive accesses, but let's be - * on the safe side. We can't cast dev_unregistering to - * volatile, because it has no effect we need, and can't cast - * it to *(volatile bool*)&, because it isn't possible to get - * address of a bit field. - */ - barrier(); } virt_dev = dev->dh_priv; diff --git a/scst/src/scst_main.c b/scst/src/scst_main.c index f604aca6a..068bfc0c3 100644 --- a/scst/src/scst_main.c +++ b/scst/src/scst_main.c @@ -1307,8 +1307,6 @@ static void scst_unregister_device(struct scsi_device *scsidp) goto out_unlock; } - dev->dev_unregistering = 1; - list_del_init(&dev->dev_list_entry); #ifdef CONFIG_SCST_FORWARD_MODE_PASS_THROUGH @@ -1535,8 +1533,7 @@ out: return res; out_unreg: - dev->dev_unregistering = 1; - list_del(&dev->dev_list_entry); + list_del_init(&dev->dev_list_entry); scst_assign_dev_handler(dev, &scst_null_devtype); goto out_pr_clear_dev; @@ -1593,8 +1590,6 @@ void scst_unregister_virtual_device(int id, goto out_unlock; } - dev->dev_unregistering = 1; - scst_cm_on_dev_unregister(dev); list_del_init(&dev->dev_list_entry); From 656931dfcea0abb61fa8179eefabaadf5f3bff0d Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 5 Jan 2019 21:55:26 +0000 Subject: [PATCH 2/3] scst_lib: Fix SCSI pass-through error handling Some but not all SCSI LLD drivers set req->errors. Some SCSI LLD drivers set req->errors to a negative Unix error code and others assign the result of make_status_bytes() to req->errors. The SCSI core finishes failed pass-through requests by calling blk_finish_request(). That function calls req->end_io() without setting req->errors. Hence check both the error argument and req->errors before calling sioc->done(). git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7861 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/dev_handlers/scst_disk.c | 6 ++++-- scst/src/scst_lib.c | 9 ++++++++- scst/src/scst_targ.c | 2 ++ 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/scst/src/dev_handlers/scst_disk.c b/scst/src/dev_handlers/scst_disk.c index 222630ca8..01120b90b 100644 --- a/scst/src/dev_handlers/scst_disk.c +++ b/scst/src/dev_handlers/scst_disk.c @@ -377,11 +377,13 @@ static void disk_cmd_done(void *data, char *sense, int result, int resid) TRACE_DBG("work %p, cmd %p, left %d, result %d, sense %p, resid %d", work, work->cmd, work->left, result, sense, resid); - work->result = result; + WARN_ON_ONCE(IS_ERR_VALUE((long)result)); - if (result == SAM_STAT_GOOD) + if (status_byte(result) == GOOD) goto out_complete; + work->result = result; + disk_restore_sg(work); scst_pass_through_cmd_done(work->cmd, sense, result, resid + work->left); diff --git a/scst/src/scst_lib.c b/scst/src/scst_lib.c index 58974cfcd..4210ac811 100644 --- a/scst/src/scst_lib.c +++ b/scst/src/scst_lib.c @@ -8411,13 +8411,20 @@ static void scsi_end_async(struct request *req, blk_status_t error) #endif if (sioc->done) { - int result, resid_len; + int resid_len; + long result; #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 12, 0) result = scsi_req(req)->result; #else result = req->errors; #endif + TRACE_DBG("error %d / %ld", error, result); + + result = result && !IS_ERR_VALUE(result) ? result : + IS_ERR_VALUE(result) || error ? + SAM_STAT_CHECK_CONDITION : 0; + #if LINUX_VERSION_CODE >= KERNEL_VERSION(4, 11, 0) resid_len = scsi_req(req)->resid_len; #elif LINUX_VERSION_CODE >= KERNEL_VERSION(2, 6, 31) diff --git a/scst/src/scst_targ.c b/scst/src/scst_targ.c index 6d48f1163..31d73c41e 100644 --- a/scst/src/scst_targ.c +++ b/scst/src/scst_targ.c @@ -2132,6 +2132,8 @@ static void scst_do_cmd_done(struct scst_cmd *cmd, int result, { TRACE_ENTRY(); + WARN_ON_ONCE(IS_ERR_VALUE((long)result)); + cmd->status = result & 0xff; cmd->msg_status = msg_byte(result); cmd->host_status = host_byte(result); From a77563897a60447ed6964631995eefeeb17d1393 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Sat, 5 Jan 2019 21:56:37 +0000 Subject: [PATCH 3/3] scst: Avoid that a high I/O load prevents activity to be suspended git-svn-id: http://svn.code.sf.net/p/scst/svn/trunk@7862 d57e44dd-8a1f-0410-8b47-8ef2f437770f --- scst/src/scst_priv.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/scst/src/scst_priv.h b/scst/src/scst_priv.h index d617ed949..063a7f083 100644 --- a/scst/src/scst_priv.h +++ b/scst/src/scst_priv.h @@ -21,6 +21,7 @@ #include #include +#include #if LINUX_VERSION_CODE >= KERNEL_VERSION(3, 2, 0) #include #endif @@ -773,6 +774,14 @@ int scst_get_suspend_count(void); static inline atomic_t *scst_get(void) { atomic_t *a; + + /* + * Avoid that a high I/O load prevents activity to be suspended. See + * also http://sourceforge.net/p/scst/mailman/message/34074831/. + */ + if (unlikely(test_bit(SCST_FLAG_SUSPENDING, &scst_flags))) + mdelay(100); + /* * We don't mind if we because of preemption inc counter from another * CPU as soon in the majority cases we will the correct one.