From patchwork Fri Dec 31 02:08:21 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529451 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id CB3A1C433F5 for ; Fri, 31 Dec 2021 02:16:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242552AbhLaCQM (ORCPT ); Thu, 30 Dec 2021 21:16:12 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46077 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242539AbhLaCQK (ORCPT ); Thu, 30 Dec 2021 21:16:10 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 486E72041CF; Fri, 31 Dec 2021 03:08:38 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PK5KtUTGyZE2; Fri, 31 Dec 2021 03:08:33 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id A519B204172; Fri, 31 Dec 2021 03:08:32 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, Luis Chamberlain Subject: [PATCH 1/9] scsi_debug: address races following module load Date: Thu, 30 Dec 2021 21:08:21 -0500 Message-Id: <20211231020829.29147-2-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org When scsi_debug is loaded as a module with many (simulated) hosts, targets, and devices (LUs), modprobe can take a long time to return. Only a small amount of this time is spent in the scsi_debug_init(); the rest is other parts of the kernel reacting to to the appearance of new storage devices. As soon as scsi_debug_init() has completed the user space may call 'rmmod scsi_debug' and this was found to cause race problems as outlined here: https://bugzilla.kernel.org/show_bug.cgi?id=212337 To reliably generate this race a sysfs parameter called rm_all_hosts was added and the code was strengthened in this area. The main change was to make the count of scsi_debug hosts present an atomic. Then it was found that the handling of the existing add_host parameter needed the same strengthening. Further: 'echo -9999 > /sys/bus/pseudo/drivers/scsi_debug/add_host has the same effect as rm_all_hosts so rm_all_hosts was not needed. To inhibit a race between two invocations of writes to add_host, a mutex was added. Also address a possible race when rmmod is called but LUs are still being added. The logic to remove (all) hosts is rather crude: it works backwards down a linked lists of hosts. Any pending requests are terminated with DID_NO_CONNECT as are any new requests. In the case where not all hosts are being removed, the ones that remain may have lost requests as just outlined. The lowest numbered host (id) hosts will remain. This patch was developed several months ago after discussions with Luis Chamberlain. It may have been overtaken by more recent work by Luis. Cc: Luis Chamberlain Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 197 ++++++++++++++++++++++++++++---------- 1 file changed, 146 insertions(+), 51 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 2104973a35cd..48b44ea2ab57 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -730,7 +731,9 @@ static const struct opcode_info_t opcode_info_arr[SDEB_I_LAST_ELEM_P1 + 1] = { {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0} }, }; -static int sdebug_num_hosts; +static atomic_t sdebug_num_hosts; +static DEFINE_MUTEX(add_host_mutex); + static int sdebug_add_host = DEF_NUM_HOST; /* in sysfs this is relative */ static int sdebug_ato = DEF_ATO; static int sdebug_cdb_len = DEF_CDB_LEN; @@ -777,6 +780,7 @@ static int sdebug_uuid_ctl = DEF_UUID_CTL; static bool sdebug_random = DEF_RANDOM; static bool sdebug_per_host_store = DEF_PER_HOST_STORE; static bool sdebug_removable = DEF_REMOVABLE; +static bool sdebug_deflect_incoming; static bool sdebug_clustering; static bool sdebug_host_lock = DEF_HOST_LOCK; static bool sdebug_strict = DEF_STRICT; @@ -5026,6 +5030,10 @@ static int scsi_debug_slave_configure(struct scsi_device *sdp) sdp->host->host_no, sdp->channel, sdp->id, sdp->lun); if (sdp->host->max_cmd_len != SDEBUG_MAX_CMD_LEN) sdp->host->max_cmd_len = SDEBUG_MAX_CMD_LEN; + if (READ_ONCE(sdebug_deflect_incoming)) { + pr_info("Exit early due to deflect_incoming\n"); + return 1; + } if (devip == NULL) { devip = find_build_dev_info(sdp); if (devip == NULL) @@ -5111,7 +5119,7 @@ static bool stop_queued_cmnd(struct scsi_cmnd *cmnd) } /* Deletes (stops) timers or work queues of all queued commands */ -static void stop_all_queued(void) +static void stop_all_queued(bool done_with_no_conn) { unsigned long iflags; int j, k; @@ -5120,13 +5128,15 @@ static void stop_all_queued(void) struct sdebug_queued_cmd *sqcp; struct sdebug_dev_info *devip; struct sdebug_defer *sd_dp; + struct scsi_cmnd *scp; for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) { spin_lock_irqsave(&sqp->qc_lock, iflags); for (k = 0; k < SDEBUG_CANQUEUE; ++k) { if (test_bit(k, sqp->in_use_bm)) { sqcp = &sqp->qc_arr[k]; - if (sqcp->a_cmnd == NULL) + scp = sqcp->a_cmnd; + if (!scp) continue; devip = (struct sdebug_dev_info *) sqcp->a_cmnd->device->hostdata; @@ -5141,6 +5151,10 @@ static void stop_all_queued(void) l_defer_t = SDEB_DEFER_NONE; spin_unlock_irqrestore(&sqp->qc_lock, iflags); stop_qc_helper(sd_dp, l_defer_t); + if (done_with_no_conn && l_defer_t != SDEB_DEFER_NONE) { + scp->result = DID_NO_CONNECT << 16; + scsi_done(scp); + } clear_bit(k, sqp->in_use_bm); spin_lock_irqsave(&sqp->qc_lock, iflags); } @@ -5283,7 +5297,7 @@ static int scsi_debug_host_reset(struct scsi_cmnd *SCpnt) } } spin_unlock(&sdebug_host_list_lock); - stop_all_queued(); + stop_all_queued(false); if (SDEBUG_OPT_RESET_NOISE & sdebug_opts) sdev_printk(KERN_INFO, SCpnt->device, "%s: %d device(s) found\n", __func__, k); @@ -5343,13 +5357,50 @@ static void sdebug_build_parts(unsigned char *ramp, unsigned long store_size) } } -static void block_unblock_all_queues(bool block) +static void sdeb_block_all_queues(void) +{ + int j; + struct sdebug_queue *sqp; + + for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) + atomic_set(&sqp->blocked, (int)true); +} + +static void sdeb_unblock_all_queues(void) { int j; struct sdebug_queue *sqp; for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) - atomic_set(&sqp->blocked, (int)block); + atomic_set(&sqp->blocked, (int)false); +} + +static void +sdeb_add_n_hosts(int num_hosts) +{ + if (num_hosts < 1) + return; + do { + bool found; + unsigned long idx; + struct sdeb_store_info *sip; + bool want_phs = (sdebug_fake_rw == 0) && sdebug_per_host_store; + + found = false; + if (want_phs) { + xa_for_each_marked(per_store_ap, idx, sip, SDEB_XA_NOT_IN_USE) { + sdeb_most_recent_idx = (int)idx; + found = true; + break; + } + if (found) /* re-use case */ + sdebug_add_host_helper((int)idx); + else + sdebug_do_add_host(true /* make new store */); + } else { + sdebug_do_add_host(false); + } + } while (--num_hosts); } /* Adjust (by rounding down) the sdebug_cmnd_count so abs(every_nth)-1 @@ -5362,10 +5413,10 @@ static void tweak_cmnd_count(void) modulo = abs(sdebug_every_nth); if (modulo < 2) return; - block_unblock_all_queues(true); + sdeb_block_all_queues(); count = atomic_read(&sdebug_cmnd_count); atomic_set(&sdebug_cmnd_count, (count / modulo) * modulo); - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); } static void clear_queue_stats(void) @@ -5383,6 +5434,15 @@ static bool inject_on_this_cmd(void) return (atomic_read(&sdebug_cmnd_count) % abs(sdebug_every_nth)) == 0; } +static int process_deflect_incoming(struct scsi_cmnd *scp) +{ + u8 opcode = scp->cmnd[0]; + + if (opcode == SYNCHRONIZE_CACHE || opcode == SYNCHRONIZE_CACHE_16) + return 0; + return DID_NO_CONNECT << 16; +} + #define INCLUSIVE_TIMING_MAX_NS 1000000 /* 1 millisecond */ /* Complete the processing of the thread that queued a SCSI command to this @@ -5392,8 +5452,7 @@ static bool inject_on_this_cmd(void) */ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, int scsi_result, - int (*pfp)(struct scsi_cmnd *, - struct sdebug_dev_info *), + int (*pfp)(struct scsi_cmnd *, struct sdebug_dev_info *), int delta_jiff, int ndelay) { bool new_sd_dp; @@ -5414,13 +5473,27 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, } sdp = cmnd->device; - if (delta_jiff == 0) + if (delta_jiff == 0) { + sqp = get_queue(cmnd); + if (atomic_read(&sqp->blocked)) { + if (READ_ONCE(sdebug_deflect_incoming)) + return process_deflect_incoming(cmnd); + else + return SCSI_MLQUEUE_HOST_BUSY; + } goto respond_in_thread; + } sqp = get_queue(cmnd); spin_lock_irqsave(&sqp->qc_lock, iflags); if (unlikely(atomic_read(&sqp->blocked))) { spin_unlock_irqrestore(&sqp->qc_lock, iflags); + if (READ_ONCE(sdebug_deflect_incoming)) { + scsi_result = process_deflect_incoming(cmnd); + goto respond_in_thread; + } + if (sdebug_verbose) + pr_info("blocked --> SCSI_MLQUEUE_HOST_BUSY\n"); return SCSI_MLQUEUE_HOST_BUSY; } num_in_q = atomic_read(&devip->num_in_q); @@ -5616,8 +5689,12 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, respond_in_thread: /* call back to mid-layer using invocation thread */ cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0; cmnd->result &= ~SDEG_RES_IMMED_MASK; - if (cmnd->result == 0 && scsi_result != 0) + if (cmnd->result == 0 && scsi_result != 0) { cmnd->result = scsi_result; + if (sdebug_verbose) + pr_info("respond_in_thread: tag=0x%x, scp->result=0x%x\n", + blk_mq_unique_tag(scsi_cmd_to_rq(cmnd)), scsi_result); + } scsi_done(cmnd); return 0; } @@ -5900,7 +5977,7 @@ static ssize_t delay_store(struct device_driver *ddp, const char *buf, int j, k; struct sdebug_queue *sqp; - block_unblock_all_queues(true); + sdeb_block_all_queues(); for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) { k = find_first_bit(sqp->in_use_bm, @@ -5914,7 +5991,7 @@ static ssize_t delay_store(struct device_driver *ddp, const char *buf, sdebug_jdelay = jdelay; sdebug_ndelay = 0; } - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); } return res; } @@ -5940,7 +6017,7 @@ static ssize_t ndelay_store(struct device_driver *ddp, const char *buf, int j, k; struct sdebug_queue *sqp; - block_unblock_all_queues(true); + sdeb_block_all_queues(); for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) { k = find_first_bit(sqp->in_use_bm, @@ -5955,7 +6032,7 @@ static ssize_t ndelay_store(struct device_driver *ddp, const char *buf, sdebug_jdelay = ndelay ? JDELAY_OVERRIDDEN : DEF_JDELAY; } - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); } return res; } @@ -6269,7 +6346,7 @@ static ssize_t max_queue_store(struct device_driver *ddp, const char *buf, if ((count > 0) && (1 == sscanf(buf, "%d", &n)) && (n > 0) && (n <= SDEBUG_CANQUEUE) && (sdebug_host_max_queue == 0)) { - block_unblock_all_queues(true); + sdeb_block_all_queues(); k = 0; for (j = 0, sqp = sdebug_q_arr; j < submit_queues; ++j, ++sqp) { @@ -6284,7 +6361,7 @@ static ssize_t max_queue_store(struct device_driver *ddp, const char *buf, atomic_set(&retired_max_queue, k + 1); else atomic_set(&retired_max_queue, 0); - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); return count; } return -EINVAL; @@ -6356,43 +6433,48 @@ static DRIVER_ATTR_RW(virtual_gb); static ssize_t add_host_show(struct device_driver *ddp, char *buf) { /* absolute number of hosts currently active is what is shown */ - return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_num_hosts); + return scnprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&sdebug_num_hosts)); } +/* + * Accept positive and negative values. Hex values (only positive) may be prefixed by '0x'. + * To remove all hosts use a large negative number (e.g. -9999). The value 0 does nothing. + * Returns -EBUSY if another add_host sysfs invocation is active. + */ static ssize_t add_host_store(struct device_driver *ddp, const char *buf, size_t count) { - bool found; - unsigned long idx; - struct sdeb_store_info *sip; - bool want_phs = (sdebug_fake_rw == 0) && sdebug_per_host_store; int delta_hosts; - if (sscanf(buf, "%d", &delta_hosts) != 1) + if (count == 0 || kstrtoint(buf, 0, &delta_hosts)) return -EINVAL; + if (sdebug_verbose) + pr_info("prior num_hosts=%d, num_to_add=%d\n", + atomic_read(&sdebug_num_hosts), delta_hosts); + if (delta_hosts == 0) + return count; + if (mutex_trylock(&add_host_mutex) == 0) + return -EBUSY; if (delta_hosts > 0) { - do { - found = false; - if (want_phs) { - xa_for_each_marked(per_store_ap, idx, sip, - SDEB_XA_NOT_IN_USE) { - sdeb_most_recent_idx = (int)idx; - found = true; - break; - } - if (found) /* re-use case */ - sdebug_add_host_helper((int)idx); - else - sdebug_do_add_host(true); - } else { - sdebug_do_add_host(false); - } - } while (--delta_hosts); + sdeb_add_n_hosts(delta_hosts); } else if (delta_hosts < 0) { + WRITE_ONCE(sdebug_deflect_incoming, true); + sdeb_block_all_queues(); + if (delta_hosts >= atomic_read(&sdebug_num_hosts)) + stop_all_queued(true); do { + if (atomic_read(&sdebug_num_hosts) < 1) { + free_all_queued(); + break; + } sdebug_do_remove_host(false); } while (++delta_hosts); + sdeb_unblock_all_queues(); + WRITE_ONCE(sdebug_deflect_incoming, false); } + mutex_unlock(&add_host_mutex); + if (sdebug_verbose) + pr_info("post num_hosts=%d\n", atomic_read(&sdebug_num_hosts)); return count; } static DRIVER_ATTR_RW(add_host); @@ -6902,6 +6984,10 @@ static int __init scsi_debug_init(void) sdebug_add_host = 0; for (k = 0; k < hosts_to_add; k++) { + if (READ_ONCE(sdebug_deflect_incoming)) { + pr_info("exit early as sdebug_deflect_incoming is set\n"); + return 0; + } if (want_store && k == 0) { ret = sdebug_add_host_helper(idx); if (ret < 0) { @@ -6919,8 +7005,12 @@ static int __init scsi_debug_init(void) } } if (sdebug_verbose) - pr_info("built %d host(s)\n", sdebug_num_hosts); + pr_info("built %d host(s)\n", atomic_read(&sdebug_num_hosts)); + /* + * Even though all the hosts have been established, due to async device (LU) scanning + * by the scsi mid-level, there may still be devices (LUs) being set up. + */ return 0; bus_unreg: @@ -6936,12 +7026,17 @@ static int __init scsi_debug_init(void) static void __exit scsi_debug_exit(void) { - int k = sdebug_num_hosts; + int k; - stop_all_queued(); - for (; k; k--) + /* Possible race with LUs still being set up; stop them asap */ + sdeb_block_all_queues(); + WRITE_ONCE(sdebug_deflect_incoming, true); + stop_all_queued(false); + for (k = 0; atomic_read(&sdebug_num_hosts) > 0; k++) sdebug_do_remove_host(true); free_all_queued(); + if (sdebug_verbose) + pr_info("removed %d hosts\n", k); driver_unregister(&sdebug_driverfs_driver); bus_unregister(&pseudo_lld_bus); root_device_unregister(pseudo_primary); @@ -7111,13 +7206,13 @@ static int sdebug_add_host_helper(int per_host_idx) sdbg_host->dev.bus = &pseudo_lld_bus; sdbg_host->dev.parent = pseudo_primary; sdbg_host->dev.release = &sdebug_release_adapter; - dev_set_name(&sdbg_host->dev, "adapter%d", sdebug_num_hosts); + dev_set_name(&sdbg_host->dev, "adapter%d", atomic_read(&sdebug_num_hosts)); error = device_register(&sdbg_host->dev); if (error) goto clean; - ++sdebug_num_hosts; + atomic_inc(&sdebug_num_hosts); return 0; clean: @@ -7181,7 +7276,7 @@ static void sdebug_do_remove_host(bool the_end) return; device_unregister(&sdbg_host->dev); - --sdebug_num_hosts; + atomic_dec(&sdebug_num_hosts); } static int sdebug_change_qdepth(struct scsi_device *sdev, int qdepth) @@ -7189,10 +7284,10 @@ static int sdebug_change_qdepth(struct scsi_device *sdev, int qdepth) int num_in_q = 0; struct sdebug_dev_info *devip; - block_unblock_all_queues(true); + sdeb_block_all_queues(); devip = (struct sdebug_dev_info *)sdev->hostdata; if (NULL == devip) { - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); return -ENODEV; } num_in_q = atomic_read(&devip->num_in_q); @@ -7211,7 +7306,7 @@ static int sdebug_change_qdepth(struct scsi_device *sdev, int qdepth) sdev_printk(KERN_INFO, sdev, "%s: qdepth=%d, num_in_q=%d\n", __func__, qdepth, num_in_q); } - block_unblock_all_queues(false); + sdeb_unblock_all_queues(); return sdev->queue_depth; } From patchwork Fri Dec 31 02:08:22 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529736 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 495EBC433EF for ; Fri, 31 Dec 2021 02:16:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242544AbhLaCQK (ORCPT ); Thu, 30 Dec 2021 21:16:10 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46078 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242534AbhLaCQJ (ORCPT ); Thu, 30 Dec 2021 21:16:09 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 0888520414C; Fri, 31 Dec 2021 03:08:40 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id lF-hqJLtYN98; Fri, 31 Dec 2021 03:08:38 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id A54042041AF; Fri, 31 Dec 2021 03:08:33 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 2/9] scsi_debug: strengthen defer_t accesses Date: Thu, 30 Dec 2021 21:08:22 -0500 Message-Id: <20211231020829.29147-3-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Use READ_ONCE() and WRITE_ONCE() macros when accessing the sdebug_defer::defer_t value. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 48b44ea2ab57..3fb9e0072627 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -4782,7 +4782,7 @@ static void sdebug_q_cmd_complete(struct sdebug_defer *sd_dp) return; } spin_lock_irqsave(&sqp->qc_lock, iflags); - sd_dp->defer_t = SDEB_DEFER_NONE; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE); sqcp = &sqp->qc_arr[qc_idx]; scp = sqcp->a_cmnd; if (unlikely(scp == NULL)) { @@ -5103,8 +5103,8 @@ static bool stop_queued_cmnd(struct scsi_cmnd *cmnd) sqcp->a_cmnd = NULL; sd_dp = sqcp->sd_dp; if (sd_dp) { - l_defer_t = sd_dp->defer_t; - sd_dp->defer_t = SDEB_DEFER_NONE; + l_defer_t = READ_ONCE(sd_dp->defer_t); + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE); } else l_defer_t = SDEB_DEFER_NONE; spin_unlock_irqrestore(&sqp->qc_lock, iflags); @@ -5145,8 +5145,8 @@ static void stop_all_queued(bool done_with_no_conn) sqcp->a_cmnd = NULL; sd_dp = sqcp->sd_dp; if (sd_dp) { - l_defer_t = sd_dp->defer_t; - sd_dp->defer_t = SDEB_DEFER_NONE; + l_defer_t = READ_ONCE(sd_dp->defer_t); + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE); } else l_defer_t = SDEB_DEFER_NONE; spin_unlock_irqrestore(&sqp->qc_lock, iflags); @@ -5627,7 +5627,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sd_dp->sqa_idx = sqp - sdebug_q_arr; sd_dp->qc_idx = k; } - sd_dp->defer_t = SDEB_DEFER_POLL; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_POLL); spin_unlock_irqrestore(&sqp->qc_lock, iflags); } else { if (!sd_dp->init_hrt) { @@ -5639,7 +5639,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sd_dp->sqa_idx = sqp - sdebug_q_arr; sd_dp->qc_idx = k; } - sd_dp->defer_t = SDEB_DEFER_HRT; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_HRT); /* schedule the invocation of scsi_done() for a later time */ hrtimer_start(&sd_dp->hrt, kt, HRTIMER_MODE_REL_PINNED); } @@ -5658,7 +5658,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sd_dp->sqa_idx = sqp - sdebug_q_arr; sd_dp->qc_idx = k; } - sd_dp->defer_t = SDEB_DEFER_POLL; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_POLL); spin_unlock_irqrestore(&sqp->qc_lock, iflags); } else { if (!sd_dp->init_wq) { @@ -5668,7 +5668,7 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, sd_dp->qc_idx = k; INIT_WORK(&sd_dp->ew.work, sdebug_q_cmd_wq_complete); } - sd_dp->defer_t = SDEB_DEFER_WQ; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_WQ); schedule_work(&sd_dp->ew.work); } if (sdebug_statistics) @@ -7436,7 +7436,7 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) queue_num, qc_idx, __func__); break; } - if (sd_dp->defer_t == SDEB_DEFER_POLL) { + if (READ_ONCE(sd_dp->defer_t) == SDEB_DEFER_POLL) { if (kt_from_boot < sd_dp->cmpl_ts) continue; @@ -7470,7 +7470,7 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) else atomic_set(&retired_max_queue, k + 1); } - sd_dp->defer_t = SDEB_DEFER_NONE; + WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE); spin_unlock_irqrestore(&sqp->qc_lock, iflags); scsi_done(scp); /* callback to mid level */ spin_lock_irqsave(&sqp->qc_lock, iflags); From patchwork Fri Dec 31 02:08:23 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529732 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 480E6C433FE for ; Fri, 31 Dec 2021 02:16:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242557AbhLaCQR (ORCPT ); Thu, 30 Dec 2021 21:16:17 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46083 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242548AbhLaCQM (ORCPT ); Thu, 30 Dec 2021 21:16:12 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id E58512041D7; Fri, 31 Dec 2021 03:08:40 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Bww5kC7yfqSP; Fri, 31 Dec 2021 03:08:39 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 868D62041BB; Fri, 31 Dec 2021 03:08:34 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 3/9] scsi_debug: use task set full more Date: Thu, 30 Dec 2021 21:08:23 -0500 Message-Id: <20211231020829.29147-4-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org When the internal in_use bit array in this driver is full returning SCSI_MLQUEUE_HOST_BUSY leads to the mid-level re-issueing the request which is unhelpful. Previously TASK SET FULL status was only returned if ALL_TSF [0x400] is placed in the opts variable (at load time or via sysfs). Now ignore that setting and always return TASK SET FULL when in_use array is full. Also set DID_ABORT together with TASK SET FULL so the mid-level gives up immediately. Aside: the situations addressed by this patch lead to lockups and timeouts. They have only been detected when blk_poll() is used. That mechanism is relatively new in the SCSI subsystem suggesting the mid-level may need more work in that area. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 3fb9e0072627..6d50d248ff3a 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -174,7 +174,7 @@ static const char *sdebug_version_date = "20200710"; #define SDEBUG_OPT_MAC_TIMEOUT 128 #define SDEBUG_OPT_SHORT_TRANSFER 0x100 #define SDEBUG_OPT_Q_NOISE 0x200 -#define SDEBUG_OPT_ALL_TSF 0x400 +#define SDEBUG_OPT_ALL_TSF 0x400 /* ignore */ #define SDEBUG_OPT_RARE_TSF 0x800 #define SDEBUG_OPT_N_WCE 0x1000 #define SDEBUG_OPT_RESET_NOISE 0x2000 @@ -861,7 +861,7 @@ static const int illegal_condition_result = (DID_ABORT << 16) | SAM_STAT_CHECK_CONDITION; static const int device_qfull_result = - (DID_OK << 16) | SAM_STAT_TASK_SET_FULL; + (DID_ABORT << 16) | SAM_STAT_TASK_SET_FULL; static const int condition_met_result = SAM_STAT_CONDITION_MET; @@ -5521,18 +5521,11 @@ static int schedule_resp(struct scsi_cmnd *cmnd, struct sdebug_dev_info *devip, spin_unlock_irqrestore(&sqp->qc_lock, iflags); if (scsi_result) goto respond_in_thread; - else if (SDEBUG_OPT_ALL_TSF & sdebug_opts) - scsi_result = device_qfull_result; + scsi_result = device_qfull_result; if (SDEBUG_OPT_Q_NOISE & sdebug_opts) - sdev_printk(KERN_INFO, sdp, - "%s: max_queue=%d exceeded, %s\n", - __func__, sdebug_max_queue, - (scsi_result ? "status: TASK SET FULL" : - "report: host busy")); - if (scsi_result) - goto respond_in_thread; - else - return SCSI_MLQUEUE_HOST_BUSY; + sdev_printk(KERN_INFO, sdp, "%s: max_queue=%d exceeded: TASK SET FULL\n", + __func__, sdebug_max_queue); + goto respond_in_thread; } set_bit(k, sqp->in_use_bm); atomic_inc(&devip->num_in_q); From patchwork Fri Dec 31 02:08:24 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529450 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A2C6FC433EF for ; Fri, 31 Dec 2021 02:16:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242554AbhLaCQN (ORCPT ); Thu, 30 Dec 2021 21:16:13 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46080 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242545AbhLaCQL (ORCPT ); Thu, 30 Dec 2021 21:16:11 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 8A8782041AE; Fri, 31 Dec 2021 03:08:41 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id F81zL4KjGTQR; Fri, 31 Dec 2021 03:08:40 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 6AFE02041BD; Fri, 31 Dec 2021 03:08:35 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 4/9] scsi_debug: refine sdebug_blk_mq_poll Date: Thu, 30 Dec 2021 21:08:24 -0500 Message-Id: <20211231020829.29147-5-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Refine the sdebug_blk_mq_poll() function so it only takes the spinlock on the queue when it can see one or more requests with the in_use bitmap flag set. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 6d50d248ff3a..0fe3fe0be8d6 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -7396,6 +7396,7 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) { bool first; bool retiring = false; + bool locked = false; int num_entries = 0; unsigned int qc_idx = 0; unsigned long iflags; @@ -7407,16 +7408,23 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) struct sdebug_defer *sd_dp; sqp = sdebug_q_arr + queue_num; - spin_lock_irqsave(&sqp->qc_lock, iflags); + qc_idx = find_first_bit(sqp->in_use_bm, sdebug_max_queue); + if (qc_idx >= sdebug_max_queue) + return 0; for (first = true; first || qc_idx + 1 < sdebug_max_queue; ) { + if (!locked) { + spin_lock_irqsave(&sqp->qc_lock, iflags); + locked = true; + } if (first) { - qc_idx = find_first_bit(sqp->in_use_bm, sdebug_max_queue); first = false; + if (!test_bit(qc_idx, sqp->in_use_bm)) + continue; } else { qc_idx = find_next_bit(sqp->in_use_bm, sdebug_max_queue, qc_idx + 1); } - if (unlikely(qc_idx >= sdebug_max_queue)) + if (qc_idx >= sdebug_max_queue) break; sqcp = &sqp->qc_arr[qc_idx]; @@ -7465,11 +7473,14 @@ static int sdebug_blk_mq_poll(struct Scsi_Host *shost, unsigned int queue_num) } WRITE_ONCE(sd_dp->defer_t, SDEB_DEFER_NONE); spin_unlock_irqrestore(&sqp->qc_lock, iflags); + locked = false; scsi_done(scp); /* callback to mid level */ - spin_lock_irqsave(&sqp->qc_lock, iflags); num_entries++; + if (find_first_bit(sqp->in_use_bm, sdebug_max_queue) >= sdebug_max_queue) + break; /* if no more then exit without retaking spinlock */ } - spin_unlock_irqrestore(&sqp->qc_lock, iflags); + if (locked) + spin_unlock_irqrestore(&sqp->qc_lock, iflags); if (num_entries > 0) atomic_add(num_entries, &sdeb_mq_poll_count); return num_entries; From patchwork Fri Dec 31 02:08:25 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529452 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BF268C433FE for ; Fri, 31 Dec 2021 02:16:11 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242551AbhLaCQL (ORCPT ); Thu, 30 Dec 2021 21:16:11 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46075 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229890AbhLaCQK (ORCPT ); Thu, 30 Dec 2021 21:16:10 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 2BF082041B2; Fri, 31 Dec 2021 03:08:44 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id EgW5AtFTXTfI; Fri, 31 Dec 2021 03:08:40 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 4D15C2041C0; Fri, 31 Dec 2021 03:08:36 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 5/9] scsi_debug: divide power on reset unit attention Date: Thu, 30 Dec 2021 21:08:25 -0500 Message-Id: <20211231020829.29147-6-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org To distinguish between resets sent by the SCSI mid-level error handling and newly introduced devices (LUs), this Unit Attention: power on, reset, or bus reset occurred [0x29,0x0] has been subdivided into that UA for the reset case and this new UA: power on occurred [0x29,0x1] for the new device (LU) case. This makes debug a little easier to follow when it is turned on (e.g. 'echo 0x1 > opts'). Bump driver version number. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 0fe3fe0be8d6..c2c82aa5b98f 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -7,7 +7,7 @@ * anything out of the ordinary is seen. * ^^^^^^^^^^^^^^^^^^^^^^^ Original ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ * - * Copyright (C) 2001 - 2020 Douglas Gilbert + * Copyright (C) 2001 - 2021 Douglas Gilbert * * For documentation see http://sg.danny.cz/sg/scsi_debug.html */ @@ -61,8 +61,8 @@ #include "scsi_logging.h" /* make sure inq_product_rev string corresponds to this version */ -#define SDEBUG_VERSION "0190" /* format to fit INQUIRY revision field */ -static const char *sdebug_version_date = "20200710"; +#define SDEBUG_VERSION "0191" /* format to fit INQUIRY revision field */ +static const char *sdebug_version_date = "20210520"; #define MY_NAME "scsi_debug" @@ -84,6 +84,7 @@ static const char *sdebug_version_date = "20200710"; #define INSUFF_RES_ASC 0x55 #define INSUFF_RES_ASCQ 0x3 #define POWER_ON_RESET_ASCQ 0x0 +#define POWER_ON_OCCURRED_ASCQ 0x1 #define BUS_RESET_ASCQ 0x2 /* scsi bus reset occurred */ #define MODE_CHANGED_ASCQ 0x1 /* mode parameters changed */ #define CAPACITY_CHANGED_ASCQ 0x9 @@ -197,13 +198,14 @@ static const char *sdebug_version_date = "20200710"; * priority. The UA numbers should be a sequence starting from 0 with * SDEBUG_NUM_UAS being 1 higher than the highest numbered UA. */ #define SDEBUG_UA_POR 0 /* Power on, reset, or bus device reset */ -#define SDEBUG_UA_BUS_RESET 1 -#define SDEBUG_UA_MODE_CHANGED 2 -#define SDEBUG_UA_CAPACITY_CHANGED 3 -#define SDEBUG_UA_LUNS_CHANGED 4 -#define SDEBUG_UA_MICROCODE_CHANGED 5 /* simulate firmware change */ -#define SDEBUG_UA_MICROCODE_CHANGED_WO_RESET 6 -#define SDEBUG_NUM_UAS 7 +#define SDEBUG_UA_POOCCUR 1 /* Power on occurred */ +#define SDEBUG_UA_BUS_RESET 2 +#define SDEBUG_UA_MODE_CHANGED 3 +#define SDEBUG_UA_CAPACITY_CHANGED 4 +#define SDEBUG_UA_LUNS_CHANGED 5 +#define SDEBUG_UA_MICROCODE_CHANGED 6 /* simulate firmware change */ +#define SDEBUG_UA_MICROCODE_CHANGED_WO_RESET 7 +#define SDEBUG_NUM_UAS 8 /* when 1==SDEBUG_OPT_MEDIUM_ERR, a medium error is simulated at this * sector on read commands: */ @@ -1086,6 +1088,12 @@ static int make_ua(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) if (sdebug_verbose) cp = "power on reset"; break; + case SDEBUG_UA_POOCCUR: + mk_sense_buffer(scp, UNIT_ATTENTION, UA_RESET_ASC, + POWER_ON_OCCURRED_ASCQ); + if (sdebug_verbose) + cp = "power on occurred"; + break; case SDEBUG_UA_BUS_RESET: mk_sense_buffer(scp, UNIT_ATTENTION, UA_RESET_ASC, BUS_RESET_ASCQ); @@ -5007,7 +5015,7 @@ static struct sdebug_dev_info *find_build_dev_info(struct scsi_device *sdev) open_devip->lun = sdev->lun; open_devip->sdbg_host = sdbg_host; atomic_set(&open_devip->num_in_q, 0); - set_bit(SDEBUG_UA_POR, open_devip->uas_bm); + set_bit(SDEBUG_UA_POOCCUR, open_devip->uas_bm); open_devip->used = true; return open_devip; } From patchwork Fri Dec 31 02:08:26 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529735 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 29862C4332F for ; Fri, 31 Dec 2021 02:16:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242550AbhLaCQL (ORCPT ); Thu, 30 Dec 2021 21:16:11 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46076 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242535AbhLaCQK (ORCPT ); Thu, 30 Dec 2021 21:16:10 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 484E02041BB; Fri, 31 Dec 2021 03:08:44 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id ooVgxkUL2KWC; Fri, 31 Dec 2021 03:08:41 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 30AD12041CB; Fri, 31 Dec 2021 03:08:37 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 6/9] scsi_debug: add no_rwlock parameter Date: Thu, 30 Dec 2021 21:08:26 -0500 Message-Id: <20211231020829.29147-7-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org By default, this driver places a read lock around all user data fetches and a write lock around all user data modifying operations (e.g. WRITE commands). These locks have "per store" granularity. Other drivers that have a similar function (e.g. null_blk) do not take this data integrity step and run significantly faster in some tests. In the common case of a (simulated) device to device copy (e.g. what dd and its variants do) there should be no need for locks around data accesses. So add the driver and sysfs parameter no_rwlock which is boolean and when set does what its name suggests. The default is false for backward comaptibility. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 152 +++++++++++++++++++++++++------------- 1 file changed, 102 insertions(+), 50 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index c2c82aa5b98f..44c74cf6b498 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -787,6 +787,7 @@ static bool sdebug_clustering; static bool sdebug_host_lock = DEF_HOST_LOCK; static bool sdebug_strict = DEF_STRICT; static bool sdebug_any_injecting_opt; +static bool sdebug_no_rwlock; static bool sdebug_verbose; static bool have_dif_prot; static bool write_since_sync; @@ -3130,6 +3131,50 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, return ret; } +static inline void +sdeb_read_lock(struct sdeb_store_info *sip) +{ + if (sdebug_no_rwlock) + return; + if (sip) + read_lock(&sip->macc_lck); + else + read_lock(&sdeb_fake_rw_lck); +} + +static inline void +sdeb_read_unlock(struct sdeb_store_info *sip) +{ + if (sdebug_no_rwlock) + return; + if (sip) + read_unlock(&sip->macc_lck); + else + read_unlock(&sdeb_fake_rw_lck); +} + +static inline void +sdeb_write_lock(struct sdeb_store_info *sip) +{ + if (sdebug_no_rwlock) + return; + if (sip) + write_lock(&sip->macc_lck); + else + write_lock(&sdeb_fake_rw_lck); +} + +static inline void +sdeb_write_unlock(struct sdeb_store_info *sip) +{ + if (sdebug_no_rwlock) + return; + if (sip) + write_unlock(&sip->macc_lck); + else + write_unlock(&sdeb_fake_rw_lck); +} + static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) { bool check_prot; @@ -3138,7 +3183,6 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) int ret; u64 lba; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; u8 *cmd = scp->cmnd; switch (cmd[0]) { @@ -3217,29 +3261,29 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) return check_condition_result; } - read_lock(macc_lckp); + sdeb_read_lock(sip); /* DIX + T10 DIF */ if (unlikely(sdebug_dix && scsi_prot_sg_count(scp))) { switch (prot_verify_read(scp, lba, num, ei_lba)) { case 1: /* Guard tag error */ if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */ - read_unlock(macc_lckp); + sdeb_read_unlock(sip); mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1); return check_condition_result; } else if (scp->prot_flags & SCSI_PROT_GUARD_CHECK) { - read_unlock(macc_lckp); + sdeb_read_unlock(sip); mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1); return illegal_condition_result; } break; case 3: /* Reference tag error */ if (cmd[1] >> 5 != 3) { /* RDPROTECT != 3 */ - read_unlock(macc_lckp); + sdeb_read_unlock(sip); mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3); return check_condition_result; } else if (scp->prot_flags & SCSI_PROT_REF_CHECK) { - read_unlock(macc_lckp); + sdeb_read_unlock(sip); mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 3); return illegal_condition_result; } @@ -3248,7 +3292,7 @@ static int resp_read_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) } ret = do_device_access(sip, scp, 0, lba, num, false); - read_unlock(macc_lckp); + sdeb_read_unlock(sip); if (unlikely(ret == -1)) return DID_ERROR << 16; @@ -3436,7 +3480,6 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) int ret; u64 lba; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; u8 *cmd = scp->cmnd; switch (cmd[0]) { @@ -3491,10 +3534,10 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) "to DIF device\n"); } - write_lock(macc_lckp); + sdeb_write_lock(sip); ret = check_device_access_params(scp, lba, num, true); if (ret) { - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return ret; } @@ -3503,22 +3546,22 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) switch (prot_verify_write(scp, lba, num, ei_lba)) { case 1: /* Guard tag error */ if (scp->prot_flags & SCSI_PROT_GUARD_CHECK) { - write_unlock(macc_lckp); + sdeb_write_unlock(sip); mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 1); return illegal_condition_result; } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */ - write_unlock(macc_lckp); + sdeb_write_unlock(sip); mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 1); return check_condition_result; } break; case 3: /* Reference tag error */ if (scp->prot_flags & SCSI_PROT_REF_CHECK) { - write_unlock(macc_lckp); + sdeb_write_unlock(sip); mk_sense_buffer(scp, ILLEGAL_REQUEST, 0x10, 3); return illegal_condition_result; } else if (scp->cmnd[1] >> 5 != 3) { /* WRPROTECT != 3 */ - write_unlock(macc_lckp); + sdeb_write_unlock(sip); mk_sense_buffer(scp, ABORTED_COMMAND, 0x10, 3); return check_condition_result; } @@ -3532,7 +3575,7 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) /* If ZBC zone then bump its write pointer */ if (sdebug_dev_is_zoned(devip)) zbc_inc_wp(devip, lba, num); - write_unlock(macc_lckp); + sdeb_write_unlock(sip); if (unlikely(-1 == ret)) return DID_ERROR << 16; else if (unlikely(sdebug_verbose && @@ -3572,7 +3615,6 @@ static int resp_write_scat(struct scsi_cmnd *scp, u8 *lrdp = NULL; u8 *up; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; u8 wrprotect; u16 lbdof, num_lrd, k; u32 num, num_by, bt_len, lbdof_blen, sg_off, cum_lb; @@ -3640,7 +3682,7 @@ static int resp_write_scat(struct scsi_cmnd *scp, goto err_out; } - write_lock(macc_lckp); + sdeb_write_lock(sip); sg_off = lbdof_blen; /* Spec says Buffer xfer Length field in number of LBs in dout */ cum_lb = 0; @@ -3722,7 +3764,7 @@ static int resp_write_scat(struct scsi_cmnd *scp, } ret = 0; err_out_unlock: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); err_out: kfree(lrdp); return ret; @@ -3739,15 +3781,14 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, int ret; struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *) scp->device->hostdata, true); - rwlock_t *macc_lckp = &sip->macc_lck; u8 *fs1p; u8 *fsp; - write_lock(macc_lckp); + sdeb_write_lock(sip); ret = check_device_access_params(scp, lba, num, true); if (ret) { - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return ret; } @@ -3767,7 +3808,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, ret = fetch_to_dev_buffer(scp, fs1p, lb_size); if (-1 == ret) { - write_unlock(&sip->macc_lck); + sdeb_write_unlock(sip); return DID_ERROR << 16; } else if (sdebug_verbose && !ndob && (ret < lb_size)) sdev_printk(KERN_INFO, scp->device, @@ -3786,7 +3827,7 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, if (sdebug_dev_is_zoned(devip)) zbc_inc_wp(devip, lba, num); out: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return 0; } @@ -3899,7 +3940,6 @@ static int resp_comp_write(struct scsi_cmnd *scp, u8 *cmd = scp->cmnd; u8 *arr; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; u64 lba; u32 dnum; u32 lb_size = sdebug_sector_size; @@ -3932,7 +3972,7 @@ static int resp_comp_write(struct scsi_cmnd *scp, return check_condition_result; } - write_lock(macc_lckp); + sdeb_write_lock(sip); ret = do_dout_fetch(scp, dnum, arr); if (ret == -1) { @@ -3950,7 +3990,7 @@ static int resp_comp_write(struct scsi_cmnd *scp, if (scsi_debug_lbp()) map_region(sip, lba, num); cleanup: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); kfree(arr); return retval; } @@ -3966,7 +4006,6 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) unsigned char *buf; struct unmap_block_desc *desc; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; unsigned int i, payload_len, descriptors; int ret; @@ -3995,7 +4034,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) desc = (void *)&buf[8]; - write_lock(macc_lckp); + sdeb_write_lock(sip); for (i = 0 ; i < descriptors ; i++) { unsigned long long lba = get_unaligned_be64(&desc[i].lba); @@ -4011,7 +4050,7 @@ static int resp_unmap(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) ret = 0; out: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); kfree(buf); return ret; @@ -4103,7 +4142,6 @@ static int resp_pre_fetch(struct scsi_cmnd *scp, u32 nblks; u8 *cmd = scp->cmnd; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; u8 *fsp = sip->storep; if (cmd[0] == PRE_FETCH) { /* 10 byte cdb */ @@ -4125,12 +4163,12 @@ static int resp_pre_fetch(struct scsi_cmnd *scp, rest = block + nblks - sdebug_store_sectors; /* Try to bring the PRE-FETCH range into CPU's cache */ - read_lock(macc_lckp); + sdeb_read_lock(sip); prefetch_range(fsp + (sdebug_sector_size * block), (nblks - rest) * sdebug_sector_size); if (rest) prefetch_range(fsp, rest * sdebug_sector_size); - read_unlock(macc_lckp); + sdeb_read_unlock(sip); fini: if (cmd[1] & 0x2) res = SDEG_RES_IMMED_MASK; @@ -4251,7 +4289,6 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) u8 *arr; u8 *cmd = scp->cmnd; struct sdeb_store_info *sip = devip2sip(devip, true); - rwlock_t *macc_lckp = &sip->macc_lck; bytchk = (cmd[1] >> 1) & 0x3; if (bytchk == 0) { @@ -4290,7 +4327,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) return check_condition_result; } /* Not changing store, so only need read access */ - read_lock(macc_lckp); + sdeb_read_lock(sip); ret = do_dout_fetch(scp, a_num, arr); if (ret == -1) { @@ -4312,7 +4349,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) goto cleanup; } cleanup: - read_unlock(macc_lckp); + sdeb_read_unlock(sip); kfree(arr); return ret; } @@ -4332,7 +4369,6 @@ static int resp_report_zones(struct scsi_cmnd *scp, u8 *cmd = scp->cmnd; struct sdeb_zone_state *zsp; struct sdeb_store_info *sip = devip2sip(devip, false); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; if (!sdebug_dev_is_zoned(devip)) { mk_sense_invalid_opcode(scp); @@ -4361,7 +4397,7 @@ static int resp_report_zones(struct scsi_cmnd *scp, return check_condition_result; } - read_lock(macc_lckp); + sdeb_read_lock(sip); desc = arr + 64; for (i = 0; i < max_zones; i++) { @@ -4449,7 +4485,7 @@ static int resp_report_zones(struct scsi_cmnd *scp, ret = fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len)); fini: - read_unlock(macc_lckp); + sdeb_read_unlock(sip); kfree(arr); return ret; } @@ -4475,14 +4511,13 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) struct sdeb_zone_state *zsp; bool all = cmd[14] & 0x01; struct sdeb_store_info *sip = devip2sip(devip, false); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; if (!sdebug_dev_is_zoned(devip)) { mk_sense_invalid_opcode(scp); return check_condition_result; } - write_lock(macc_lckp); + sdeb_write_lock(sip); if (all) { /* Check if all closed zones can be open */ @@ -4531,7 +4566,7 @@ static int resp_open_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) zbc_open_zone(devip, zsp, true); fini: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return res; } @@ -4552,14 +4587,13 @@ static int resp_close_zone(struct scsi_cmnd *scp, struct sdeb_zone_state *zsp; bool all = cmd[14] & 0x01; struct sdeb_store_info *sip = devip2sip(devip, false); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; if (!sdebug_dev_is_zoned(devip)) { mk_sense_invalid_opcode(scp); return check_condition_result; } - write_lock(macc_lckp); + sdeb_write_lock(sip); if (all) { zbc_close_all(devip); @@ -4588,7 +4622,7 @@ static int resp_close_zone(struct scsi_cmnd *scp, zbc_close_zone(devip, zsp); fini: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return res; } @@ -4625,14 +4659,13 @@ static int resp_finish_zone(struct scsi_cmnd *scp, u8 *cmd = scp->cmnd; bool all = cmd[14] & 0x01; struct sdeb_store_info *sip = devip2sip(devip, false); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; if (!sdebug_dev_is_zoned(devip)) { mk_sense_invalid_opcode(scp); return check_condition_result; } - write_lock(macc_lckp); + sdeb_write_lock(sip); if (all) { zbc_finish_all(devip); @@ -4661,7 +4694,7 @@ static int resp_finish_zone(struct scsi_cmnd *scp, zbc_finish_zone(devip, zsp, true); fini: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return res; } @@ -4706,14 +4739,13 @@ static int resp_rwp_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) u8 *cmd = scp->cmnd; bool all = cmd[14] & 0x01; struct sdeb_store_info *sip = devip2sip(devip, false); - rwlock_t *macc_lckp = sip ? &sip->macc_lck : &sdeb_fake_rw_lck; if (!sdebug_dev_is_zoned(devip)) { mk_sense_invalid_opcode(scp); return check_condition_result; } - write_lock(macc_lckp); + sdeb_write_lock(sip); if (all) { zbc_rwp_all(devip); @@ -4741,7 +4773,7 @@ static int resp_rwp_zone(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) zbc_rwp_zone(devip, zsp); fini: - write_unlock(macc_lckp); + sdeb_write_unlock(sip); return res; } @@ -5740,6 +5772,7 @@ module_param_named(medium_error_start, sdebug_medium_error_start, int, S_IRUGO | S_IWUSR); module_param_named(ndelay, sdebug_ndelay, int, S_IRUGO | S_IWUSR); module_param_named(no_lun_0, sdebug_no_lun_0, int, S_IRUGO | S_IWUSR); +module_param_named(no_rwlock, sdebug_no_rwlock, bool, S_IRUGO | S_IWUSR); module_param_named(no_uld, sdebug_no_uld, int, S_IRUGO); module_param_named(num_parts, sdebug_num_parts, int, S_IRUGO); module_param_named(num_tgts, sdebug_num_tgts, int, S_IRUGO | S_IWUSR); @@ -5812,6 +5845,7 @@ MODULE_PARM_DESC(medium_error_count, "count of sectors to return follow on MEDIU MODULE_PARM_DESC(medium_error_start, "starting sector number to return MEDIUM error"); MODULE_PARM_DESC(ndelay, "response delay in nanoseconds (def=0 -> ignore)"); MODULE_PARM_DESC(no_lun_0, "no LU number 0 (def=0 -> have lun 0)"); +MODULE_PARM_DESC(no_rwlock, "don't protect user data reads+writes (def=0)"); MODULE_PARM_DESC(no_uld, "stop ULD (e.g. sd driver) attaching (def=0))"); MODULE_PARM_DESC(num_parts, "number of partitions(def=0)"); MODULE_PARM_DESC(num_tgts, "number of targets per host to simulate(def=1)"); @@ -6374,6 +6408,23 @@ static ssize_t host_max_queue_show(struct device_driver *ddp, char *buf) return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_host_max_queue); } +static ssize_t no_rwlock_show(struct device_driver *ddp, char *buf) +{ + return scnprintf(buf, PAGE_SIZE, "%d\n", sdebug_no_rwlock); +} + +static ssize_t no_rwlock_store(struct device_driver *ddp, const char *buf, size_t count) +{ + bool v; + + if (kstrtobool(buf, &v)) + return -EINVAL; + + sdebug_no_rwlock = v; + return count; +} +static DRIVER_ATTR_RW(no_rwlock); + /* * Since this is used for .can_queue, and we get the hc_idx tag from the bitmap * in range [0, sdebug_host_max_queue), we can't change it. @@ -6739,6 +6790,7 @@ static struct attribute *sdebug_drv_attrs[] = { &driver_attr_lun_format.attr, &driver_attr_max_luns.attr, &driver_attr_max_queue.attr, + &driver_attr_no_rwlock.attr, &driver_attr_no_uld.attr, &driver_attr_scsi_level.attr, &driver_attr_virtual_gb.attr, From patchwork Fri Dec 31 02:08:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529733 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5608C433F5 for ; Fri, 31 Dec 2021 02:16:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242555AbhLaCQP (ORCPT ); Thu, 30 Dec 2021 21:16:15 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46082 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242547AbhLaCQM (ORCPT ); Thu, 30 Dec 2021 21:16:12 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id E25CB2041BD; Fri, 31 Dec 2021 03:08:45 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 7CHrElf+TvdR; Fri, 31 Dec 2021 03:08:44 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 3E8472041CE; Fri, 31 Dec 2021 03:08:38 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com, Bodo Stroesser Subject: [PATCH 7/9] scsi_debug: add sdeb_sgl_copy_sgl and friends Date: Thu, 30 Dec 2021 21:08:27 -0500 Message-Id: <20211231020829.29147-8-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org The scatter-gather list helper functions in this patch have been proposed for inclusion in lib/scatterlist.c where they would be exported. Before that happens they need to be accepted via another kernel subsystem maintainer. In the meantime they are placed in this driver with a "sdeb_" prefix and with static scope. The next patch in this set makes extensive use of these functions as the backing store(s) of the scsi_debug driver is changed from vmalloc() based to sgl based. These functions where proposed for the patchset whose cover was titled: "[PATCH v4 0/4] scatterlist: add new capabilities". That patchset was sent to the linux-block, linux-scsi and linux-kernel lists on 20201016. Since all patches in that set were reviewed by Bodo Stroesser his review is carried over to this patch. Reviewed-by: Bodo Stroesser Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 127 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 44c74cf6b498..464467fbbf1a 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -1206,6 +1206,133 @@ static int p_fill_from_dev_buffer(struct scsi_cmnd *scp, const void *arr, return 0; } +/* + * The following 4 functions are proposed for lib/scatterlist.c (without the + * "sdeb_" prefix) and will be removed from this driver if the proposal is accepted. + */ +static size_t sdeb_sgl_copy_sgl(struct scatterlist *d_sgl, unsigned int d_nents, off_t d_skip, + struct scatterlist *s_sgl, unsigned int s_nents, off_t s_skip, + size_t n_bytes) +{ + size_t len; + size_t offset = 0; + struct sg_mapping_iter d_iter, s_iter; + + if (n_bytes == 0) + return 0; + sg_miter_start(&s_iter, s_sgl, s_nents, SG_MITER_ATOMIC | SG_MITER_FROM_SG); + sg_miter_start(&d_iter, d_sgl, d_nents, SG_MITER_ATOMIC | SG_MITER_TO_SG); + if (!sg_miter_skip(&s_iter, s_skip)) + goto fini; + if (!sg_miter_skip(&d_iter, d_skip)) + goto fini; + + while (offset < n_bytes) { + if (!sg_miter_next(&s_iter)) + break; + if (!sg_miter_next(&d_iter)) + break; + len = min3(d_iter.length, s_iter.length, n_bytes - offset); + + memcpy(d_iter.addr, s_iter.addr, len); + offset += len; + /* LIFO order (stop d_iter before s_iter) needed with SG_MITER_ATOMIC */ + d_iter.consumed = len; + sg_miter_stop(&d_iter); + s_iter.consumed = len; + sg_miter_stop(&s_iter); + } +fini: + sg_miter_stop(&d_iter); + sg_miter_stop(&s_iter); + return offset; +} + +static bool sdeb_sgl_compare_sgl_idx(struct scatterlist *x_sgl, unsigned int x_nents, off_t x_skip, + struct scatterlist *y_sgl, unsigned int y_nents, off_t y_skip, + size_t n_bytes, size_t *miscompare_idx) +{ + bool equ = true; + size_t len; + size_t offset = 0; + struct sg_mapping_iter x_iter, y_iter; + + if (n_bytes == 0) + return true; + sg_miter_start(&x_iter, x_sgl, x_nents, SG_MITER_ATOMIC | SG_MITER_FROM_SG); + sg_miter_start(&y_iter, y_sgl, y_nents, SG_MITER_ATOMIC | SG_MITER_FROM_SG); + if (!sg_miter_skip(&x_iter, x_skip)) + goto fini; + if (!sg_miter_skip(&y_iter, y_skip)) + goto fini; + + while (offset < n_bytes) { + if (!sg_miter_next(&x_iter)) + break; + if (!sg_miter_next(&y_iter)) + break; + len = min3(x_iter.length, y_iter.length, n_bytes - offset); + + equ = !memcmp(x_iter.addr, y_iter.addr, len); + if (!equ) + goto fini; + offset += len; + /* LIFO order is important when SG_MITER_ATOMIC is used */ + y_iter.consumed = len; + sg_miter_stop(&y_iter); + x_iter.consumed = len; + sg_miter_stop(&x_iter); + } +fini: + if (miscompare_idx && !equ) { + u8 *xp = x_iter.addr; + u8 *yp = y_iter.addr; + u8 *x_endp; + + for (x_endp = xp + len ; xp < x_endp; ++xp, ++yp) { + if (*xp != *yp) + break; + } + *miscompare_idx = offset + len - (x_endp - xp); + } + sg_miter_stop(&y_iter); + sg_miter_stop(&x_iter); + return equ; +} + +static bool sdeb_sgl_compare_sgl(struct scatterlist *x_sgl, unsigned int x_nents, off_t x_skip, + struct scatterlist *y_sgl, unsigned int y_nents, off_t y_skip, + size_t n_bytes) +{ + return sdeb_sgl_compare_sgl_idx(x_sgl, x_nents, x_skip, y_sgl, y_nents, y_skip, n_bytes, + NULL); +} + +static size_t sdeb_sgl_memset(struct scatterlist *sgl, unsigned int nents, off_t skip, + u8 val, size_t n_bytes) +{ + size_t offset = 0; + size_t len; + struct sg_mapping_iter miter; + + if (n_bytes == 0) + return 0; + sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_TO_SG); + if (!sg_miter_skip(&miter, skip)) + goto fini; + + while ((offset < n_bytes) && sg_miter_next(&miter)) { + len = min(miter.length, n_bytes - offset); + memset(miter.addr, val, len); + offset += len; + } +fini: + sg_miter_stop(&miter); + return offset; +} + +/* end of functions proposed for lib/scatterlist.c */ + /* Fetches from SCSI "data-out" buffer. Returns number of bytes fetched into * 'arr' or -1 if error. */ From patchwork Fri Dec 31 02:08:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529449 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6B41BC433EF for ; Fri, 31 Dec 2021 02:16:17 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242556AbhLaCQR (ORCPT ); Thu, 30 Dec 2021 21:16:17 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46084 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242549AbhLaCQM (ORCPT ); Thu, 30 Dec 2021 21:16:12 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 59C5E2041AF; Fri, 31 Dec 2021 03:08:47 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id dT7yDEVat46h; Fri, 31 Dec 2021 03:08:44 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 4174F2041D4; Fri, 31 Dec 2021 03:08:39 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 8/9] scsi_debug: change store from vmalloc to sgl Date: Thu, 30 Dec 2021 21:08:28 -0500 Message-Id: <20211231020829.29147-9-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org A long time ago this driver's store was allocated by kmalloc() or alloc_pages(). When this was switched to vmalloc() the author noticed slower ramdisk access times and more variability in repeated tests. So try going back with sgl_alloc_order() to get uniformly sized allocations in a sometimes large scatter gather _array_. That array is the basis of maintaining O(1) access to the store. Using sgl_alloc_order() and friends requires CONFIG_SGL_ALLOC so add a 'select' to the Kconfig file. Remove kcalloc() in resp_verify() as sgl_s can now be compared directly without forming an intermediate buffer. This is a performance win for the SCSI VERIFY command implementation. Make the SCSI COMPARE AND WRITE command yield the offset of the first miscompared byte when the compare fails (as required by T10). This patch previously depended on: "[PATCH v4 0/4] scatterlist: add new capabilities". However while that patchset is being considered, those functions have been replicated by the previous patch with a "sdeb_" prefix so they can be used by this patch. Signed-off-by: Douglas Gilbert --- drivers/scsi/Kconfig | 3 +- drivers/scsi/scsi_debug.c | 491 ++++++++++++++++++++++++++------------ 2 files changed, 347 insertions(+), 147 deletions(-) diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig index 6e3a04107bb6..39bd9ee2b07e 100644 --- a/drivers/scsi/Kconfig +++ b/drivers/scsi/Kconfig @@ -1240,13 +1240,14 @@ config SCSI_DEBUG tristate "SCSI debugging host and device simulator" depends on SCSI select CRC_T10DIF + select SGL_ALLOC help This pseudo driver simulates one or more hosts (SCSI initiators), each with one or more targets, each with one or more logical units. Defaults to one of each, creating a small RAM disk device. Many parameters found in the /sys/bus/pseudo/drivers/scsi_debug directory can be tweaked at run time. - See for more information. + See for more information. Mainly used for testing and best as a module. If unsure, say N. config SCSI_MESH diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 464467fbbf1a..ba4a5a7dd25f 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -222,6 +222,7 @@ static const char *sdebug_version_date = "20210520"; #define SDEBUG_CANQUEUE_WORDS 3 /* a WORD is bits in a long */ #define SDEBUG_CANQUEUE (SDEBUG_CANQUEUE_WORDS * BITS_PER_LONG) #define DEF_CMD_PER_LUN SDEBUG_CANQUEUE +#define SDEB_ORDER_TOO_LARGE 4096 /* UA - Unit Attention; SA - Service Action; SSU - Start Stop Unit */ #define F_D_IN 1 /* Data-in command (e.g. READ) */ @@ -315,8 +316,11 @@ struct sdebug_host_info { /* There is an xarray of pointers to this struct's objects, one per host */ struct sdeb_store_info { + unsigned int n_elem; /* number of sgl elements */ + unsigned int order; /* as used by alloc_pages() */ + unsigned int elem_pow2; /* PAGE_SHIFT + order */ rwlock_t macc_lck; /* for atomic media access on this store */ - u8 *storep; /* user data storage (ram) */ + struct scatterlist *sgl; /* main store: n_elem array of same sized allocs */ struct t10_pi_tuple *dif_storep; /* protection info */ void *map_storep; /* provisioning map */ }; @@ -879,19 +883,6 @@ static inline bool scsi_debug_lbp(void) (sdebug_lbpu || sdebug_lbpws || sdebug_lbpws10); } -static void *lba2fake_store(struct sdeb_store_info *sip, - unsigned long long lba) -{ - struct sdeb_store_info *lsip = sip; - - lba = do_div(lba, sdebug_store_sectors); - if (!sip || !sip->storep) { - WARN_ON_ONCE(true); - lsip = xa_load(per_store_ap, 0); /* should never be NULL */ - } - return lsip->storep + lba * sdebug_sector_size; -} - static struct t10_pi_tuple *dif_store(struct sdeb_store_info *sip, sector_t sector) { @@ -1000,7 +991,6 @@ static int scsi_debug_ioctl(struct scsi_device *dev, unsigned int cmd, __func__, cmd); } return -EINVAL; - /* return -ENOTTY; // correct return but upsets fdisk */ } static void config_cdb_len(struct scsi_device *sdev) @@ -1206,6 +1196,38 @@ static int p_fill_from_dev_buffer(struct scsi_cmnd *scp, const void *arr, return 0; } +static struct scatterlist *sdeb_sgl_alloc_order(unsigned long long length, unsigned int order, + gfp_t gfp, unsigned int *nent_p) +{ + struct scatterlist *sgl, *sg; + struct page *page; + unsigned int nent, nalloc; + u32 elem_len; + + nent = round_up(length, PAGE_SIZE << order) >> (PAGE_SHIFT + order); + nalloc = nent; + sgl = kmalloc_array(nalloc, sizeof(struct scatterlist), gfp & ~GFP_DMA); + if (!sgl) + return NULL; + + sg_init_table(sgl, nalloc); + sg = sgl; + while (length) { + elem_len = min_t(u64, length, PAGE_SIZE << order); + page = alloc_pages(gfp, order); + if (!page) { + sgl_free_order(sgl, order); + return NULL; + } + sg_set_page(sg, page, elem_len, 0); + length -= elem_len; + sg = sg_next(sg); + } + if (nent_p) + *nent_p = nent; + return sgl; +} + /* * The following 4 functions are proposed for lib/scatterlist.c (without the * "sdeb_" prefix) and will be removed from this driver if the proposal is accepted. @@ -1300,13 +1322,15 @@ static bool sdeb_sgl_compare_sgl_idx(struct scatterlist *x_sgl, unsigned int x_n return equ; } -static bool sdeb_sgl_compare_sgl(struct scatterlist *x_sgl, unsigned int x_nents, off_t x_skip, - struct scatterlist *y_sgl, unsigned int y_nents, off_t y_skip, - size_t n_bytes) -{ - return sdeb_sgl_compare_sgl_idx(x_sgl, x_nents, x_skip, y_sgl, y_nents, y_skip, n_bytes, - NULL); -} +/* + * static bool sdeb_sgl_compare_sgl(struct scatterlist *x_sgl, unsigned int x_nents, off_t x_skip, + * struct scatterlist *y_sgl, unsigned int y_nents, off_t y_skip, + * size_t n_bytes) + * { + * return sdeb_sgl_compare_sgl_idx(x_sgl, x_nents, x_skip, y_sgl, y_nents, y_skip, n_bytes, + * NULL); + * } + */ static size_t sdeb_sgl_memset(struct scatterlist *sgl, unsigned int nents, off_t skip, u8 val, size_t n_bytes) @@ -1347,6 +1371,54 @@ static int fetch_to_dev_buffer(struct scsi_cmnd *scp, unsigned char *arr, return scsi_sg_copy_to_buffer(scp, arr, arr_len); } +static bool sdeb_sgl_cmp_buf(struct scatterlist *sgl, unsigned int nents, + const void *buf, size_t buflen, off_t skip) +{ + bool equ = true; + size_t offset = 0; + size_t len; + struct sg_mapping_iter miter; + + if (buflen == 0) + return true; + sg_miter_start(&miter, sgl, nents, SG_MITER_ATOMIC | SG_MITER_FROM_SG); + if (!sg_miter_skip(&miter, skip)) + goto fini; + + while (equ && (offset < buflen) && sg_miter_next(&miter)) { + len = min(miter.length, buflen - offset); + equ = memcmp(buf + offset, miter.addr, len) == 0; + offset += len; + miter.consumed = len; + sg_miter_stop(&miter); + } +fini: + sg_miter_stop(&miter); + return equ; +} + +static void sdeb_sgl_prefetch(struct scatterlist *sgl, unsigned int nents, + off_t skip, size_t n_bytes) +{ + size_t offset = 0; + size_t len; + struct sg_mapping_iter miter; + unsigned int sg_flags = SG_MITER_FROM_SG; + + if (n_bytes == 0) + return; + sg_miter_start(&miter, sgl, nents, sg_flags); + if (!sg_miter_skip(&miter, skip)) + goto fini; + + while ((offset < n_bytes) && sg_miter_next(&miter)) { + len = min(miter.length, n_bytes - offset); + prefetch_range(miter.addr, len); + offset += len; + } +fini: + sg_miter_stop(&miter); +} static char sdebug_inq_vendor_id[9] = "Linux "; static char sdebug_inq_product_id[17] = "scsi_debug "; @@ -3046,13 +3118,14 @@ static inline struct sdeb_store_info *devip2sip(struct sdebug_dev_info *devip, /* Returns number of bytes copied or -1 if error. */ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, - u32 sg_skip, u64 lba, u32 num, bool do_write) + u32 data_inout_off, u64 lba, u32 n_blks, bool do_write) { int ret; - u64 block, rest = 0; + u32 lb_size = sdebug_sector_size; + u64 block, sgl_i, rem, lba_start, rest = 0; enum dma_data_direction dir; struct scsi_data_buffer *sdb = &scp->sdb; - u8 *fsp; + struct scatterlist *store_sgl; if (do_write) { dir = DMA_TO_DEVICE; @@ -3065,25 +3138,38 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp, return 0; if (scp->sc_data_direction != dir) return -1; - fsp = sip->storep; - block = do_div(lba, sdebug_store_sectors); - if (block + num > sdebug_store_sectors) - rest = block + num - sdebug_store_sectors; + if (block + n_blks > sdebug_store_sectors) + rest = block + n_blks - sdebug_store_sectors; + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; /* O(1) to each store sg element */ + + if (do_write) + ret = sdeb_sgl_copy_sgl(store_sgl, sip->n_elem - sgl_i, rem, + sdb->table.sgl, sdb->table.nents, data_inout_off, + (n_blks - rest) * lb_size); + else + ret = sdeb_sgl_copy_sgl(sdb->table.sgl, sdb->table.nents, data_inout_off, + store_sgl, sip->n_elem - sgl_i, rem, + (n_blks - rest) * lb_size); - ret = sg_copy_buffer(sdb->table.sgl, sdb->table.nents, - fsp + (block * sdebug_sector_size), - (num - rest) * sdebug_sector_size, sg_skip, do_write); - if (ret != (num - rest) * sdebug_sector_size) + if (ret != (n_blks - rest) * lb_size) return ret; - if (rest) { - ret += sg_copy_buffer(sdb->table.sgl, sdb->table.nents, - fsp, rest * sdebug_sector_size, - sg_skip + ((num - rest) * sdebug_sector_size), - do_write); - } - + if (rest == 0) + goto fini; + if (do_write) + ret += sdeb_sgl_copy_sgl(sip->sgl, sip->n_elem, 0, sdb->table.sgl, + sdb->table.nents, + data_inout_off + ((n_blks - rest) * lb_size), + rest * lb_size); + else + ret += sdeb_sgl_copy_sgl(sdb->table.sgl, sdb->table.nents, + data_inout_off + ((n_blks - rest) * lb_size), + sip->sgl, sip->n_elem, 0, rest * lb_size); +fini: return ret; } @@ -3100,37 +3186,66 @@ static int do_dout_fetch(struct scsi_cmnd *scp, u32 num, u8 *doutp) num * sdebug_sector_size, 0, true); } -/* If sip->storep+lba compares equal to arr(num), then copy top half of - * arr into sip->storep+lba and return true. If comparison fails then - * return false. */ +/* If sip->storep+lba compares equal to arr(num) or scp->sdb, then if miscomp_idxp is non-NULL, + * copy top half of arr into sip->storep+lba and return true. If comparison fails then return + * false and write the miscompare_idx via miscomp_idxp. This is the COMAPARE AND WRITE case. + * For VERIFY(BytChk=1), set arr to NULL which causes a sgl (store) to sgl (data-out buffer) + * compare to be done. VERIFY(BytChk=3) sets arr to a valid address and sets miscomp_idxp + * to NULL. + */ static bool comp_write_worker(struct sdeb_store_info *sip, u64 lba, u32 num, - const u8 *arr, bool compare_only) + const u8 *arr, struct scsi_cmnd *scp, size_t *miscomp_idxp) { - bool res; - u64 block, rest = 0; + bool equ; + u64 block, lba_start, sgl_i, rem, rest = 0; u32 store_blks = sdebug_store_sectors; - u32 lb_size = sdebug_sector_size; - u8 *fsp = sip->storep; + const u32 lb_size = sdebug_sector_size; + u32 top_half = num * lb_size; + struct scsi_data_buffer *sdb = &scp->sdb; + struct scatterlist *store_sgl; block = do_div(lba, store_blks); if (block + num > store_blks) rest = block + num - store_blks; + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; /* O(1) to each store sg element */ + + if (!arr) { /* sgl to sgl compare */ + equ = sdeb_sgl_compare_sgl_idx(store_sgl, sip->n_elem - sgl_i, rem, + sdb->table.sgl, sdb->table.nents, 0, + (num - rest) * lb_size, miscomp_idxp); + if (!equ) + return equ; + if (rest > 0) + equ = sdeb_sgl_compare_sgl_idx(sip->sgl, sip->n_elem, 0, sdb->table.sgl, + sdb->table.nents, (num - rest) * lb_size, + rest * lb_size, miscomp_idxp); + } else { + equ = sdeb_sgl_cmp_buf(store_sgl, sip->n_elem - sgl_i, arr, + (num - rest) * lb_size, 0); + if (!equ) + return equ; + if (rest > 0) + equ = sdeb_sgl_cmp_buf(sip->sgl, sip->n_elem, arr, + (num - rest) * lb_size, 0); + } + if (!equ || !miscomp_idxp) + return equ; - res = !memcmp(fsp + (block * lb_size), arr, (num - rest) * lb_size); - if (!res) - return res; - if (rest) - res = memcmp(fsp, arr + ((num - rest) * lb_size), - rest * lb_size); - if (!res) - return res; - if (compare_only) - return true; - arr += num * lb_size; - memcpy(fsp + (block * lb_size), arr, (num - rest) * lb_size); - if (rest) - memcpy(fsp, arr + ((num - rest) * lb_size), rest * lb_size); - return res; + /* Copy "top half" of dout (args: 4, 5 and 6) into store sgl (args 1, 2 and 3) */ + sdeb_sgl_copy_sgl(store_sgl, sip->n_elem - sgl_i, rem, + sdb->table.sgl, sdb->table.nents, top_half, + (num - rest) * lb_size); + if (rest > 0) { /* for virtual_gb need to handle wrap-around of store */ + u32 src_off = top_half + ((num - rest) * lb_size); + + sdeb_sgl_copy_sgl(sip->sgl, sip->n_elem, 0, + sdb->table.sgl, sdb->table.nents, src_off, + rest * lb_size); + } + return true; } static __be16 dif_compute_csum(const void *buf, int len) @@ -3223,13 +3338,22 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, { int ret = 0; unsigned int i; + const u32 lb_size = sdebug_sector_size; sector_t sector; + u64 lba, lba_start, block, rem, sgl_i; struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *) scp->device->hostdata, true); struct t10_pi_tuple *sdt; + struct scatterlist *store_sgl; + u8 *arr; + + arr = kzalloc(lb_size, GFP_ATOMIC); + if (!arr) + return -1; /* mkp, is this correct? */ for (i = 0; i < sectors; i++, ei_lba++) { sector = start_sec + i; + lba = sector; sdt = dif_store(sip, sector); if (sdt->app_tag == cpu_to_be16(0xffff)) @@ -3243,11 +3367,19 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, * have to iterate over the PI twice. */ if (scp->cmnd[1] >> 5) { /* RDPROTECT */ - ret = dif_verify(sdt, lba2fake_store(sip, sector), - sector, ei_lba); + block = do_div(lba, sdebug_store_sectors); + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; + + ret = sg_copy_buffer(store_sgl, sip->n_elem - sgl_i, arr, lb_size, rem, true); + + ret = dif_verify(sdt, arr, sector, ei_lba); + if (ret) { dif_errors++; - break; + goto fini; } } } @@ -3255,6 +3387,8 @@ static int prot_verify_read(struct scsi_cmnd *scp, sector_t start_sec, dif_copy_prot(scp, start_sec, sectors, true); dix_reads++; +fini: + kfree(arr); return ret; } @@ -3449,6 +3583,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, unsigned int sectors, u32 ei_lba) { int ret; + const u32 lb_size = sdebug_sector_size; struct t10_pi_tuple *sdt; void *daddr; sector_t sector = start_sec; @@ -3498,7 +3633,7 @@ static int prot_verify_write(struct scsi_cmnd *SCpnt, sector_t start_sec, sector++; ei_lba++; - dpage_offset += sdebug_sector_size; + dpage_offset += lb_size; } diter.consumed = dpage_offset; sg_miter_stop(&diter); @@ -3573,8 +3708,8 @@ static void map_region(struct sdeb_store_info *sip, sector_t lba, static void unmap_region(struct sdeb_store_info *sip, sector_t lba, unsigned int len) { + const u32 lb_size = sdebug_sector_size; sector_t end = lba + len; - u8 *fsp = sip->storep; while (lba < end) { unsigned long index = lba_to_map_index(lba); @@ -3584,10 +3719,26 @@ static void unmap_region(struct sdeb_store_info *sip, sector_t lba, index < map_size) { clear_bit(index, sip->map_storep); if (sdebug_lbprz) { /* for LBPRZ=2 return 0xff_s */ - memset(fsp + lba * sdebug_sector_size, - (sdebug_lbprz & 1) ? 0 : 0xff, - sdebug_sector_size * - sdebug_unmap_granularity); + int val = (sdebug_lbprz & 1) ? 0 : 0xff; + u32 num = sdebug_unmap_granularity; + u64 lbaa = lba; + u64 rest = 0; + u64 block, lba_start, sgl_i, rem; + struct scatterlist *store_sgl; + + block = do_div(lbaa, sdebug_store_sectors); + if (block + num > sdebug_store_sectors) + rest = block + num - sdebug_store_sectors; + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; + + sdeb_sgl_memset(store_sgl, sip->n_elem - sgl_i, rem, val, + num * lb_size); + if (rest) + sdeb_sgl_memset(sip->sgl, sip->n_elem, rem, val, + (num - rest) * lb_size); } if (sip->dif_storep) { memset(sip->dif_storep + lba, 0xff, @@ -3745,7 +3896,7 @@ static int resp_write_scat(struct scsi_cmnd *scp, u8 wrprotect; u16 lbdof, num_lrd, k; u32 num, num_by, bt_len, lbdof_blen, sg_off, cum_lb; - u32 lb_size = sdebug_sector_size; + const u32 lb_size = sdebug_sector_size; u32 ei_lba; u64 lba; int ret, res; @@ -3903,13 +4054,12 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, struct scsi_device *sdp = scp->device; struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata; unsigned long long i; - u64 block, lbaa; - u32 lb_size = sdebug_sector_size; + u64 block, lbaa, sgl_i, lba_start, rem; + const u32 lb_size = sdebug_sector_size; int ret; struct sdeb_store_info *sip = devip2sip((struct sdebug_dev_info *) scp->device->hostdata, true); - u8 *fs1p; - u8 *fsp; + struct scatterlist *store_sgl; sdeb_write_lock(sip); @@ -3925,15 +4075,17 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, } lbaa = lba; block = do_div(lbaa, sdebug_store_sectors); - /* if ndob then zero 1 logical block, else fetch 1 logical block */ - fsp = sip->storep; - fs1p = fsp + (block * lb_size); - if (ndob) { - memset(fs1p, 0, lb_size); - ret = 0; - } else - ret = fetch_to_dev_buffer(scp, fs1p, lb_size); + /* if ndob then zero 1 logical block, else fetch 1 logical block */ + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; + ret = 0; + if (ndob) + sdeb_sgl_memset(store_sgl, sip->n_elem - sgl_i, rem, 0, lb_size); + else + ret = do_device_access(sip, scp, 0, lba, 1, true); if (-1 == ret) { sdeb_write_unlock(sip); return DID_ERROR << 16; @@ -3944,9 +4096,11 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, /* Copy first sector to remaining blocks */ for (i = 1 ; i < num ; i++) { - lbaa = lba + i; - block = do_div(lbaa, sdebug_store_sectors); - memmove(fsp + (block * lb_size), fs1p, lb_size); + ret = do_device_access(sip, scp, 0, lba + i, 1, true); + if (-1 == ret) { + write_unlock(&sip->macc_lck); + return DID_ERROR << 16; + } } if (scsi_debug_lbp()) map_region(sip, lba, num); @@ -3955,7 +4109,6 @@ static int resp_write_same(struct scsi_cmnd *scp, u64 lba, u32 num, zbc_inc_wp(devip, lba, num); out: sdeb_write_unlock(sip); - return 0; } @@ -4061,15 +4214,14 @@ static int resp_write_buffer(struct scsi_cmnd *scp, return 0; } -static int resp_comp_write(struct scsi_cmnd *scp, - struct sdebug_dev_info *devip) +static int resp_comp_write(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) { u8 *cmd = scp->cmnd; - u8 *arr; struct sdeb_store_info *sip = devip2sip(devip, true); u64 lba; + size_t miscomp_idx; u32 dnum; - u32 lb_size = sdebug_sector_size; + const u32 lb_size = sdebug_sector_size; u8 num; int ret; int retval = 0; @@ -4092,25 +4244,21 @@ static int resp_comp_write(struct scsi_cmnd *scp, if (ret) return ret; dnum = 2 * num; - arr = kcalloc(lb_size, dnum, GFP_ATOMIC); - if (NULL == arr) { - mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, - INSUFF_RES_ASCQ); - return check_condition_result; - } sdeb_write_lock(sip); - - ret = do_dout_fetch(scp, dnum, arr); - if (ret == -1) { - retval = DID_ERROR << 16; + if (scp->sdb.length < dnum * lb_size || scp->sc_data_direction != DMA_TO_DEVICE) { + mk_sense_buffer(scp, ILLEGAL_REQUEST, PARAMETER_LIST_LENGTH_ERR, 0); + retval = check_condition_result; + if (sdebug_verbose) + sdev_printk(KERN_INFO, scp->device, + "%s::%s: cdb indicated=%u, IO sent=%d bytes\n", my_name, + __func__, dnum * lb_size, ret); goto cleanup; - } else if (sdebug_verbose && (ret < (dnum * lb_size))) - sdev_printk(KERN_INFO, scp->device, "%s: compare_write: cdb " - "indicated=%u, IO sent=%d bytes\n", my_name, - dnum * lb_size, ret); - if (!comp_write_worker(sip, lba, num, arr, false)) { + } + + if (!comp_write_worker(sip, lba, num, NULL, scp, &miscomp_idx)) { mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0); + scsi_set_sense_information(scp->sense_buffer, SCSI_SENSE_BUFFERSIZE, miscomp_idx); retval = check_condition_result; goto cleanup; } @@ -4118,7 +4266,6 @@ static int resp_comp_write(struct scsi_cmnd *scp, map_region(sip, lba, num); cleanup: sdeb_write_unlock(sip); - kfree(arr); return retval; } @@ -4264,12 +4411,12 @@ static int resp_pre_fetch(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) { int res = 0; - u64 lba; - u64 block, rest = 0; + const u32 lb_size = sdebug_sector_size; + u64 lba, block, sgl_i, rem, lba_start, rest = 0; u32 nblks; u8 *cmd = scp->cmnd; struct sdeb_store_info *sip = devip2sip(devip, true); - u8 *fsp = sip->storep; + struct scatterlist *store_sgl; if (cmd[0] == PRE_FETCH) { /* 10 byte cdb */ lba = get_unaligned_be32(cmd + 2); @@ -4282,21 +4429,21 @@ static int resp_pre_fetch(struct scsi_cmnd *scp, mk_sense_buffer(scp, ILLEGAL_REQUEST, LBA_OUT_OF_RANGE, 0); return check_condition_result; } - if (!fsp) - goto fini; /* PRE-FETCH spec says nothing about LBP or PI so skip them */ block = do_div(lba, sdebug_store_sectors); if (block + nblks > sdebug_store_sectors) rest = block + nblks - sdebug_store_sectors; + lba_start = block * lb_size; + sgl_i = lba_start >> sip->elem_pow2; + rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + store_sgl = sip->sgl + sgl_i; /* O(1) to each store sg element */ /* Try to bring the PRE-FETCH range into CPU's cache */ sdeb_read_lock(sip); - prefetch_range(fsp + (sdebug_sector_size * block), - (nblks - rest) * sdebug_sector_size); + sdeb_sgl_prefetch(store_sgl, sip->n_elem - sgl_i, rem, (nblks - rest) * lb_size); if (rest) - prefetch_range(fsp, rest * sdebug_sector_size); + sdeb_sgl_prefetch(sip->sgl, sip->n_elem, 0, rest * lb_size); sdeb_read_unlock(sip); -fini: if (cmd[1] & 0x2) res = SDEG_RES_IMMED_MASK; return res | condition_met_result; @@ -4413,7 +4560,7 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) u32 vnum, a_num, off; const u32 lb_size = sdebug_sector_size; u64 lba; - u8 *arr; + u8 *arr = NULL; u8 *cmd = scp->cmnd; struct sdeb_store_info *sip = devip2sip(devip, true); @@ -4447,30 +4594,31 @@ static int resp_verify(struct scsi_cmnd *scp, struct sdebug_dev_info *devip) if (ret) return ret; - arr = kcalloc(lb_size, vnum, GFP_ATOMIC); - if (!arr) { - mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, - INSUFF_RES_ASCQ); - return check_condition_result; + if (is_bytchk3) { + arr = kcalloc(lb_size, vnum, GFP_ATOMIC); + if (!arr) { + mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, INSUFF_RES_ASCQ); + return check_condition_result; + } } /* Not changing store, so only need read access */ sdeb_read_lock(sip); - ret = do_dout_fetch(scp, a_num, arr); - if (ret == -1) { - ret = DID_ERROR << 16; - goto cleanup; - } else if (sdebug_verbose && (ret < (a_num * lb_size))) { - sdev_printk(KERN_INFO, scp->device, - "%s: %s: cdb indicated=%u, IO sent=%d bytes\n", - my_name, __func__, a_num * lb_size, ret); - } if (is_bytchk3) { + ret = do_dout_fetch(scp, a_num, arr); + if (ret == -1) { + ret = DID_ERROR << 16; + goto cleanup; + } else if (sdebug_verbose && (ret < (a_num * lb_size))) { + sdev_printk(KERN_INFO, scp->device, + "%s: %s: cdb indicated=%u, IO sent=%d bytes\n", + my_name, __func__, a_num * lb_size, ret); + } for (j = 1, off = lb_size; j < vnum; ++j, off += lb_size) memcpy(arr + off, arr, lb_size); } ret = 0; - if (!comp_write_worker(sip, lba, vnum, arr, true)) { + if (!comp_write_worker(sip, lba, vnum, arr, scp, NULL)) { mk_sense_buffer(scp, MISCOMPARE, MISCOMPARE_VERIFY_ASC, 0); ret = check_condition_result; goto cleanup; @@ -4841,9 +4989,16 @@ static void zbc_rwp_zone(struct sdebug_dev_info *devip, if (zsp->z_cond == ZC4_CLOSED) devip->nr_closed--; - if (zsp->z_wp > zsp->z_start) - memset(sip->storep + zsp->z_start * sdebug_sector_size, 0, - (zsp->z_wp - zsp->z_start) * sdebug_sector_size); + if (zsp->z_wp > zsp->z_start) { + u32 lb_size = sdebug_sector_size; + u64 lba_start = zsp->z_start * lb_size; + u64 sgl_i = lba_start >> sip->elem_pow2; + u64 rem = lba_start - (sgl_i ? (sgl_i << sip->elem_pow2) : 0); + struct scatterlist *store_sgl = sip->sgl + sgl_i; + + sdeb_sgl_memset(store_sgl, sip->n_elem - sgl_i, rem, 0, + (zsp->z_wp - zsp->z_start) * lb_size); + } zsp->z_non_seq_resource = false; zsp->z_wp = zsp->z_start; @@ -6106,15 +6261,15 @@ static int scsi_debug_show_info(struct seq_file *m, struct Scsi_Host *host) sdhp->shost->host_no, idx); ++j; } - seq_printf(m, "\nper_store array [most_recent_idx=%d]:\n", + seq_printf(m, "\nper_store array [most_recent_idx=%d] sgl_s:\n", sdeb_most_recent_idx); j = 0; xa_for_each(per_store_ap, l_idx, sip) { niu = xa_get_mark(per_store_ap, l_idx, SDEB_XA_NOT_IN_USE); idx = (int)l_idx; - seq_printf(m, " %d: idx=%d%s\n", j, idx, - (niu ? " not_in_use" : "")); + seq_printf(m, " %d: idx=%d%s, n_elems=%u, elem_sz=%u\n", j, idx, + (niu ? " not_in_use" : ""), sip->n_elem, 1 << sip->elem_pow2); ++j; } } @@ -7251,7 +7406,8 @@ static void sdebug_erase_store(int idx, struct sdeb_store_info *sip) } vfree(sip->map_storep); vfree(sip->dif_storep); - vfree(sip->storep); + if (sip->sgl) + sgl_free_n_order(sip->sgl, sip->n_elem, sip->order); xa_erase(per_store_ap, idx); kfree(sip); } @@ -7272,6 +7428,41 @@ static void sdebug_erase_all_stores(bool apart_from_first) sdeb_most_recent_idx = sdeb_first_idx; } +/* Want uniform sg element size, the last one can be less. */ +static int sdeb_store_sgat(struct sdeb_store_info *sip, int sz_mib) +{ + unsigned int order; + unsigned long sz_b = (unsigned long)sz_mib * 1048576; + gfp_t mask_ap = GFP_KERNEL | __GFP_COMP | __GFP_NOWARN | __GFP_ZERO; + + if (sz_mib <= 128) + order = get_order(max_t(unsigned int, PAGE_SIZE, 32 * 1024)); + else if (sz_mib <= 256) + order = get_order(max_t(unsigned int, PAGE_SIZE, 64 * 1024)); + else if (sz_mib <= 512) + order = get_order(max_t(unsigned int, PAGE_SIZE, 128 * 1024)); + else if (sz_mib <= 1024) + order = get_order(max_t(unsigned int, PAGE_SIZE, 256 * 1024)); + else if (sz_mib <= 2048) + order = get_order(max_t(unsigned int, PAGE_SIZE, 512 * 1024)); + else + order = get_order(max_t(unsigned int, PAGE_SIZE, 1024 * 1024)); + sip->sgl = sdeb_sgl_alloc_order(sz_b, order, mask_ap, &sip->n_elem); + if (!sip->sgl && order > 0) { + sip->sgl = sdeb_sgl_alloc_order(sz_b, --order, mask_ap, &sip->n_elem); + if (!sip->sgl && order > 0) + sip->sgl = sdeb_sgl_alloc_order(sz_b, --order, mask_ap, &sip->n_elem); + } + if (!sip->sgl) { + pr_info("%s: unable to obtain %d MiB, last element size: %u kiB\n", __func__, + sz_mib, (1 << (PAGE_SHIFT + order)) / 1024); + return -ENOMEM; + } + sip->order = order; + sip->elem_pow2 = PAGE_SHIFT + order; + return 0; +} + /* * Returns store xarray new element index (idx) if >=0 else negated errno. * Limit the number of stores to 65536. @@ -7303,13 +7494,21 @@ static int sdebug_add_store(void) xa_unlock_irqrestore(per_store_ap, iflags); res = -ENOMEM; - sip->storep = vzalloc(sz); - if (!sip->storep) { - pr_err("user data oom\n"); + res = sdeb_store_sgat(sip, sdebug_dev_size_mb); + if (res) { + pr_err("sgat: user data oom\n"); goto err; } - if (sdebug_num_parts > 0) - sdebug_build_parts(sip->storep, sz); + if (sdebug_num_parts > 0) { + const int a_len = 1024; + u8 *arr = kzalloc(a_len, GFP_KERNEL); + + if (arr) { + sdebug_build_parts(arr, sz); + sg_copy_from_buffer(sip->sgl, sip->n_elem, arr, a_len); + kfree(arr); + } + } /* DIF/DIX: what T10 calls Protection Information (PI) */ if (sdebug_dix) { From patchwork Fri Dec 31 02:08:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Gilbert X-Patchwork-Id: 529734 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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DC2F3C43217 for ; Fri, 31 Dec 2021 02:16:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S242553AbhLaCQM (ORCPT ); Thu, 30 Dec 2021 21:16:12 -0500 Received: from smtp.infotech.no ([82.134.31.41]:46081 "EHLO smtp.infotech.no" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242546AbhLaCQL (ORCPT ); Thu, 30 Dec 2021 21:16:11 -0500 Received: from localhost (localhost [127.0.0.1]) by smtp.infotech.no (Postfix) with ESMTP id 1E86A2041CB; Fri, 31 Dec 2021 03:08:47 +0100 (CET) X-Virus-Scanned: by amavisd-new-2.6.6 (20110518) (Debian) at infotech.no Received: from smtp.infotech.no ([127.0.0.1]) by localhost (smtp.infotech.no [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id fFfh8VW146ie; Fri, 31 Dec 2021 03:08:45 +0100 (CET) Received: from xtwo70.bingwo.ca (host-45-58-208-241.dyn.295.ca [45.58.208.241]) by smtp.infotech.no (Postfix) with ESMTPA id 49EF02041AF; Fri, 31 Dec 2021 03:08:40 +0100 (CET) From: Douglas Gilbert To: linux-scsi@vger.kernel.org Cc: martin.petersen@oracle.com, jejb@linux.vnet.ibm.com Subject: [PATCH 9/9] scsi_debug: add environmental reporting log subpage Date: Thu, 30 Dec 2021 21:08:29 -0500 Message-Id: <20211231020829.29147-10-dgilbert@interlog.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20211231020829.29147-1-dgilbert@interlog.com> References: <20211231020829.29147-1-dgilbert@interlog.com> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org Log subpages are starting to appear in real devices (e.g. SSDs) so add support for one. Adopt approach where all "wild" sub-pages are themselves listed as long as there is at least one non-wild page or subpage for a given page number. Signed-off-by: Douglas Gilbert --- drivers/scsi/scsi_debug.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index ba4a5a7dd25f..6e981c4b6379 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -2793,6 +2793,18 @@ static int resp_ie_l_pg(unsigned char *arr) return sizeof(ie_l_pg); } +static int resp_env_rep_l_spg(unsigned char *arr) +{ + unsigned char env_rep_l_spg[] = {0x0, 0x0, 0x23, 0x8, + 0x0, 40, 72, 0xff, 45, 18, 0, 0, + 0x1, 0x0, 0x23, 0x8, + 0x0, 55, 72, 35, 55, 45, 0, 0, + }; + + memcpy(arr, env_rep_l_spg, sizeof(env_rep_l_spg)); + return sizeof(env_rep_l_spg); +} + #define SDEBUG_MAX_LSENSE_SZ 512 static int resp_log_sense(struct scsi_cmnd *scp, @@ -2845,26 +2857,47 @@ static int resp_log_sense(struct scsi_cmnd *scp, arr[n++] = 0xff; /* this page */ arr[n++] = 0xd; arr[n++] = 0x0; /* Temperature */ + arr[n++] = 0xd; + arr[n++] = 0x1; /* Environment reporting */ + arr[n++] = 0xd; + arr[n++] = 0xff; /* all 0xd subpages */ arr[n++] = 0x2f; arr[n++] = 0x0; /* Informational exceptions */ + arr[n++] = 0x2f; + arr[n++] = 0xff; /* all 0x2f subpages */ arr[3] = n - 4; break; case 0xd: /* Temperature subpages */ n = 4; arr[n++] = 0xd; arr[n++] = 0x0; /* Temperature */ + arr[n++] = 0xd; + arr[n++] = 0x1; /* Environment reporting */ + arr[n++] = 0xd; + arr[n++] = 0xff; /* these subpages */ arr[3] = n - 4; break; case 0x2f: /* Informational exceptions subpages */ n = 4; arr[n++] = 0x2f; arr[n++] = 0x0; /* Informational exceptions */ + arr[n++] = 0x2f; + arr[n++] = 0xff; /* these subpages */ arr[3] = n - 4; break; default: mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, 5); return check_condition_result; } + } else if (subpcode > 0) { + arr[0] |= 0x40; + arr[1] = subpcode; + if (pcode == 0xd && subpcode == 1) + arr[3] = resp_env_rep_l_spg(arr + 4); + else { + mk_sense_invalid_fld(scp, SDEB_IN_CDB, 2, 5); + return check_condition_result; + } } else { mk_sense_invalid_fld(scp, SDEB_IN_CDB, 3, -1); return check_condition_result;