From f00e0d7714895d10790e2bac4df00727ddb9abff Mon Sep 17 00:00:00 2001 From: William Dean Date: Sat, 23 Jul 2022 14:40:27 +0800 Subject: scsi: lpfc: Check the return value of alloc_workqueue() The function alloc_workqueue() in lpfc_sli4_driver_resource_setup() can fail, but there is no check of its return value. The return value should be checked. Link: https://lore.kernel.org/r/20220723064027.2956623-1-williamsukatube@163.com Fixes: 3cee98db2610 ("scsi: lpfc: Fix crash on driver unload in wq free") Reported-by: Hacash Robot Reviewed-by: James Smart Signed-off-by: William Dean Signed-off-by: Martin K. Petersen --- drivers/scsi/lpfc/lpfc_init.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c index 4a0eadd1c22c..c69c5a0979ec 100644 --- a/drivers/scsi/lpfc/lpfc_init.c +++ b/drivers/scsi/lpfc/lpfc_init.c @@ -7948,6 +7948,8 @@ lpfc_sli4_driver_resource_setup(struct lpfc_hba *phba) /* The lpfc_wq workqueue for deferred irq use */ phba->wq = alloc_workqueue("lpfc_wq", WQ_MEM_RECLAIM, 0); + if (!phba->wq) + return -ENOMEM; /* * Initialize timers used by driver -- cgit v1.2.3-58-ga151 From 86a44f045b8cbfd885b7425f4cd8a1c353593057 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Tue, 26 Jul 2022 15:52:21 -0700 Subject: scsi: ufs: core: Increase the maximum data buffer size Measurements for one particular UFS controller + UFS device show a 25% higher read bandwidth if the maximum data buffer size is increased from 512 KiB to 1 MiB. Hence increase the maximum size of the data buffer associated with a single request from SCSI_DEFAULT_MAX_SECTORS (1024) * 512 bytes = 512 KiB to 1 MiB. Notes: - The maximum data buffer size supported by the UFSHCI specification is 65535 * 256 KiB or about 16 GiB. - The maximum data buffer size for READ(10) commands is 65535 logical blocks. To transfer more than 65535 * 4096 bytes = 255 MiB with a single SCSI command, the READ(16) command is required. Support for READ(16) is optional in the UFS 3.1 and UFS 4.0 standards. Link: https://lore.kernel.org/r/20220726225232.1362251-1-bvanassche@acm.org Cc: Adrian Hunter Cc: Avri Altman Cc: Bean Huo Cc: Stanley Chu Tested-by: Avri Altman Acked-by: Avri Altman Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index f4f8ded97ef2..dbe74f6b0c47 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -8304,6 +8304,7 @@ static struct scsi_host_template ufshcd_driver_template = { .cmd_per_lun = UFSHCD_CMD_PER_LUN, .can_queue = UFSHCD_CAN_QUEUE, .max_segment_size = PRDT_DATA_BYTE_COUNT_MAX, + .max_sectors = (1 << 20) / SECTOR_SIZE, /* 1 MiB */ .max_host_blocked = 1, .track_queue_depth = 1, .sdev_groups = ufshcd_driver_groups, -- cgit v1.2.3-58-ga151 From 00511d2abf5708ad05dd5d1c36adb2468d274698 Mon Sep 17 00:00:00 2001 From: Peter Wang Date: Wed, 27 Jul 2022 11:05:26 +0800 Subject: scsi: ufs: core: Correct ufshcd_shutdown() flow After ufshcd_wl_shutdown() set device power off and link off, ufshcd_shutdown() could turn off clock/power. Also remove pm_runtime_get_sync. The reason why it is safe to remove pm_runtime_get_sync() is because: - ufshcd_wl_shutdown() -> pm_runtime_get_sync() will resume hba->dev too. - device resume(turn on clk/power) is not required, even if device is in RPM_SUSPENDED. Link: https://lore.kernel.org/r/20220727030526.31022-1-peter.wang@mediatek.com Fixes: b294ff3e3449 ("scsi: ufs: core: Enable power management for wlun") Cc: # 5.15.x Reviewed-by: Stanley Chu Signed-off-by: Peter Wang Signed-off-by: Martin K. Petersen --- drivers/ufs/core/ufshcd.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index dbe74f6b0c47..0dc6437e956a 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -9487,12 +9487,8 @@ EXPORT_SYMBOL(ufshcd_runtime_resume); int ufshcd_shutdown(struct ufs_hba *hba) { if (ufshcd_is_ufs_dev_poweroff(hba) && ufshcd_is_link_off(hba)) - goto out; - - pm_runtime_get_sync(hba->dev); + ufshcd_suspend(hba); - ufshcd_suspend(hba); -out: hba->is_powered = false; /* allow force shutdown even in case of errors */ return 0; -- cgit v1.2.3-58-ga151 From dd0a66ada0bd0ae6c96ea45cfa1581797e867a40 Mon Sep 17 00:00:00 2001 From: Dmitry Bogdanov Date: Thu, 28 Jul 2022 00:41:24 +0300 Subject: scsi: target: core: Fix race during ACL removal Under huge load there is a possibility of race condition in updating se_dev_entry object in ACL removal procedure: NIP [c0080000154093d0] transport_lookup_cmd_lun+0x1f8/0x3d0 [target_core_mod] LR [c00800001542ab34] target_submit_cmd_map_sgls+0x11c/0x300 [target_core_mod] Call Trace: target_submit_cmd_map_sgls+0x11c/0x300 [target_core_mod] target_submit_cmd+0x44/0x60 [target_core_mod] tcm_qla2xxx_handle_cmd+0x88/0xe0 [tcm_qla2xxx] qlt_do_work+0x2e4/0x3d0 [qla2xxx] process_one_work+0x298/0x5c Despite usage of RCU primitives with deve->se_lun pointer, it has not become dereference-safe because deve->se_lun is updated and not synchronized with a reader. That change might be in a release function called by synchronize_rcu(). But, in fact, there is no point in setting that pointer to NULL for deleting deve. All access to deve->se_lun is already under rcu_read_lock. And either deve->se_lun is always valid or deve is not valid itself and will not be found in the list_for_*. The same applicable for deve->se_lun_acl too. So a better solution is to remove that NULLing. Link: https://lore.kernel.org/r/20220727214125.19647-2-d.bogdanov@yadro.com Reviewed-by: Mike Christie Signed-off-by: Dmitry Bogdanov Signed-off-by: Martin K. Petersen --- drivers/target/target_core_device.c | 3 --- 1 file changed, 3 deletions(-) diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 086ac9c9343c..88a844ae1fd0 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -434,9 +434,6 @@ void core_disable_device_list_for_node( kref_put(&orig->pr_kref, target_pr_kref_release); wait_for_completion(&orig->pr_comp); - rcu_assign_pointer(orig->se_lun, NULL); - rcu_assign_pointer(orig->se_lun_acl, NULL); - kfree_rcu(orig, rcu_head); core_scsi3_free_pr_reg_from_nacl(dev, nacl); -- cgit v1.2.3-58-ga151 From ef4f7e4bf1dc26aaa86cf8e5a13013684139be51 Mon Sep 17 00:00:00 2001 From: Dmitry Bogdanov Date: Thu, 28 Jul 2022 00:41:25 +0300 Subject: scsi: target: core: De-RCU of se_lun and se_lun acl se_lun and se_lun_acl are immutable pointers of struct se_dev_entry. Remove RCU usage for access to those pointers. Link: https://lore.kernel.org/r/20220727214125.19647-3-d.bogdanov@yadro.com Reviewed-by: Mike Christie Signed-off-by: Dmitry Bogdanov Signed-off-by: Martin K. Petersen --- drivers/target/target_core_alua.c | 3 +-- drivers/target/target_core_device.c | 29 +++++++++++------------------ drivers/target/target_core_pr.c | 28 ++++++++-------------------- drivers/target/target_core_stat.c | 10 ++++------ drivers/target/target_core_xcopy.c | 2 +- include/target/target_core_base.h | 4 ++-- 6 files changed, 27 insertions(+), 49 deletions(-) diff --git a/drivers/target/target_core_alua.c b/drivers/target/target_core_alua.c index 58df0145e8d0..fb91423a4e2e 100644 --- a/drivers/target/target_core_alua.c +++ b/drivers/target/target_core_alua.c @@ -934,8 +934,7 @@ static void core_alua_queue_state_change_ua(struct t10_alua_tg_pt_gp *tg_pt_gp) spin_lock(&lun->lun_deve_lock); list_for_each_entry(se_deve, &lun->lun_deve_list, lun_link) { - lacl = rcu_dereference_check(se_deve->se_lun_acl, - lockdep_is_held(&lun->lun_deve_lock)); + lacl = se_deve->se_lun_acl; /* * spc4r37 p.242: diff --git a/drivers/target/target_core_device.c b/drivers/target/target_core_device.c index 88a844ae1fd0..b7f16ee8aa0e 100644 --- a/drivers/target/target_core_device.c +++ b/drivers/target/target_core_device.c @@ -75,7 +75,7 @@ transport_lookup_cmd_lun(struct se_cmd *se_cmd) return TCM_WRITE_PROTECTED; } - se_lun = rcu_dereference(deve->se_lun); + se_lun = deve->se_lun; if (!percpu_ref_tryget_live(&se_lun->lun_ref)) { se_lun = NULL; @@ -152,7 +152,7 @@ int transport_lookup_tmr_lun(struct se_cmd *se_cmd) rcu_read_lock(); deve = target_nacl_find_deve(nacl, se_cmd->orig_fe_lun); if (deve) { - se_lun = rcu_dereference(deve->se_lun); + se_lun = deve->se_lun; if (!percpu_ref_tryget_live(&se_lun->lun_ref)) { se_lun = NULL; @@ -216,7 +216,7 @@ struct se_dev_entry *core_get_se_deve_from_rtpi( rcu_read_lock(); hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) { - lun = rcu_dereference(deve->se_lun); + lun = deve->se_lun; if (!lun) { pr_err("%s device entries device pointer is" " NULL, but Initiator has access.\n", @@ -243,11 +243,8 @@ void core_free_device_list_for_node( struct se_dev_entry *deve; mutex_lock(&nacl->lun_entry_mutex); - hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) { - struct se_lun *lun = rcu_dereference_check(deve->se_lun, - lockdep_is_held(&nacl->lun_entry_mutex)); - core_disable_device_list_for_node(lun, deve, nacl, tpg); - } + hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) + core_disable_device_list_for_node(deve->se_lun, deve, nacl, tpg); mutex_unlock(&nacl->lun_entry_mutex); } @@ -334,8 +331,7 @@ int core_enable_device_list_for_node( mutex_lock(&nacl->lun_entry_mutex); orig = target_nacl_find_deve(nacl, mapped_lun); if (orig && orig->se_lun) { - struct se_lun *orig_lun = rcu_dereference_check(orig->se_lun, - lockdep_is_held(&nacl->lun_entry_mutex)); + struct se_lun *orig_lun = orig->se_lun; if (orig_lun != lun) { pr_err("Existing orig->se_lun doesn't match new lun" @@ -355,8 +351,8 @@ int core_enable_device_list_for_node( return -EINVAL; } - rcu_assign_pointer(new->se_lun, lun); - rcu_assign_pointer(new->se_lun_acl, lun_acl); + new->se_lun = lun; + new->se_lun_acl = lun_acl; hlist_del_rcu(&orig->link); hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist); mutex_unlock(&nacl->lun_entry_mutex); @@ -374,8 +370,8 @@ int core_enable_device_list_for_node( return 0; } - rcu_assign_pointer(new->se_lun, lun); - rcu_assign_pointer(new->se_lun_acl, lun_acl); + new->se_lun = lun; + new->se_lun_acl = lun_acl; hlist_add_head_rcu(&new->link, &nacl->lun_entry_hlist); mutex_unlock(&nacl->lun_entry_mutex); @@ -454,10 +450,7 @@ void core_clear_lun_from_tpg(struct se_lun *lun, struct se_portal_group *tpg) mutex_lock(&nacl->lun_entry_mutex); hlist_for_each_entry_rcu(deve, &nacl->lun_entry_hlist, link) { - struct se_lun *tmp_lun = rcu_dereference_check(deve->se_lun, - lockdep_is_held(&nacl->lun_entry_mutex)); - - if (lun != tmp_lun) + if (lun != deve->se_lun) continue; core_disable_device_list_for_node(lun, deve, nacl, tpg); diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c index 3829b61b56c1..a1d67554709f 100644 --- a/drivers/target/target_core_pr.c +++ b/drivers/target/target_core_pr.c @@ -739,8 +739,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration( if (!deve_tmp->se_lun_acl) continue; - lacl_tmp = rcu_dereference_check(deve_tmp->se_lun_acl, - lockdep_is_held(&lun_tmp->lun_deve_lock)); + lacl_tmp = deve_tmp->se_lun_acl; nacl_tmp = lacl_tmp->se_lun_nacl; /* * Skip the matching struct se_node_acl that is allocated @@ -784,8 +783,7 @@ static struct t10_pr_registration *__core_scsi3_alloc_registration( * the original *pr_reg is processed in * __core_scsi3_add_registration() */ - dest_lun = rcu_dereference_check(deve_tmp->se_lun, - kref_read(&deve_tmp->pr_kref) != 0); + dest_lun = deve_tmp->se_lun; pr_reg_atp = __core_scsi3_do_alloc_registration(dev, nacl_tmp, dest_lun, deve_tmp, @@ -1437,34 +1435,26 @@ static void core_scsi3_nodeacl_undepend_item(struct se_node_acl *nacl) static int core_scsi3_lunacl_depend_item(struct se_dev_entry *se_deve) { - struct se_lun_acl *lun_acl; - /* * For nacl->dynamic_node_acl=1 */ - lun_acl = rcu_dereference_check(se_deve->se_lun_acl, - kref_read(&se_deve->pr_kref) != 0); - if (!lun_acl) + if (!se_deve->se_lun_acl) return 0; - return target_depend_item(&lun_acl->se_lun_group.cg_item); + return target_depend_item(&se_deve->se_lun_acl->se_lun_group.cg_item); } static void core_scsi3_lunacl_undepend_item(struct se_dev_entry *se_deve) { - struct se_lun_acl *lun_acl; - /* * For nacl->dynamic_node_acl=1 */ - lun_acl = rcu_dereference_check(se_deve->se_lun_acl, - kref_read(&se_deve->pr_kref) != 0); - if (!lun_acl) { + if (!se_deve->se_lun_acl) { kref_put(&se_deve->pr_kref, target_pr_kref_release); return; } - target_undepend_item(&lun_acl->se_lun_group.cg_item); + target_undepend_item(&se_deve->se_lun_acl->se_lun_group.cg_item); kref_put(&se_deve->pr_kref, target_pr_kref_release); } @@ -1751,8 +1741,7 @@ core_scsi3_decode_spec_i_port( * and then call __core_scsi3_add_registration() in the * 2nd loop which will never fail. */ - dest_lun = rcu_dereference_check(dest_se_deve->se_lun, - kref_read(&dest_se_deve->pr_kref) != 0); + dest_lun = dest_se_deve->se_lun; dest_pr_reg = __core_scsi3_alloc_registration(cmd->se_dev, dest_node_acl, dest_lun, dest_se_deve, @@ -3446,8 +3435,7 @@ after_iport_check: dest_pr_reg = __core_scsi3_locate_pr_reg(dev, dest_node_acl, iport_ptr); if (!dest_pr_reg) { - struct se_lun *dest_lun = rcu_dereference_check(dest_se_deve->se_lun, - kref_read(&dest_se_deve->pr_kref) != 0); + struct se_lun *dest_lun = dest_se_deve->se_lun; spin_unlock(&dev->dev_reservation_lock); if (core_scsi3_alloc_registration(cmd->se_dev, dest_node_acl, diff --git a/drivers/target/target_core_stat.c b/drivers/target/target_core_stat.c index 62d15bcc3d93..f85ee5b0fd80 100644 --- a/drivers/target/target_core_stat.c +++ b/drivers/target/target_core_stat.c @@ -877,7 +877,6 @@ static ssize_t target_stat_auth_dev_show(struct config_item *item, struct se_lun_acl *lacl = auth_to_lacl(item); struct se_node_acl *nacl = lacl->se_lun_nacl; struct se_dev_entry *deve; - struct se_lun *lun; ssize_t ret; rcu_read_lock(); @@ -886,9 +885,9 @@ static ssize_t target_stat_auth_dev_show(struct config_item *item, rcu_read_unlock(); return -ENODEV; } - lun = rcu_dereference(deve->se_lun); + /* scsiDeviceIndex */ - ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_index); + ret = snprintf(page, PAGE_SIZE, "%u\n", deve->se_lun->lun_index); rcu_read_unlock(); return ret; } @@ -1217,7 +1216,6 @@ static ssize_t target_stat_iport_dev_show(struct config_item *item, struct se_lun_acl *lacl = iport_to_lacl(item); struct se_node_acl *nacl = lacl->se_lun_nacl; struct se_dev_entry *deve; - struct se_lun *lun; ssize_t ret; rcu_read_lock(); @@ -1226,9 +1224,9 @@ static ssize_t target_stat_iport_dev_show(struct config_item *item, rcu_read_unlock(); return -ENODEV; } - lun = rcu_dereference(deve->se_lun); + /* scsiDeviceIndex */ - ret = snprintf(page, PAGE_SIZE, "%u\n", lun->lun_index); + ret = snprintf(page, PAGE_SIZE, "%u\n", deve->se_lun->lun_index); rcu_read_unlock(); return ret; } diff --git a/drivers/target/target_core_xcopy.c b/drivers/target/target_core_xcopy.c index 6bb20aa9c5bc..8713cda0c2fb 100644 --- a/drivers/target/target_core_xcopy.c +++ b/drivers/target/target_core_xcopy.c @@ -88,7 +88,7 @@ static int target_xcopy_locate_se_dev_e4(struct se_session *sess, struct se_device *this_dev; int rc; - this_lun = rcu_dereference(deve->se_lun); + this_lun = deve->se_lun; this_dev = rcu_dereference_raw(this_lun->lun_se_dev); rc = target_xcopy_locate_se_dev_e4_iter(this_dev, dev_wwn); diff --git a/include/target/target_core_base.h b/include/target/target_core_base.h index c2b36f7d917d..8c920456edd9 100644 --- a/include/target/target_core_base.h +++ b/include/target/target_core_base.h @@ -665,9 +665,9 @@ struct se_dev_entry { /* Used for PR SPEC_I_PT=1 and REGISTER_AND_MOVE */ struct kref pr_kref; struct completion pr_comp; - struct se_lun_acl __rcu *se_lun_acl; + struct se_lun_acl *se_lun_acl; spinlock_t ua_lock; - struct se_lun __rcu *se_lun; + struct se_lun *se_lun; #define DEF_PR_REG_ACTIVE 1 unsigned long deve_flags; struct list_head alua_port_list; -- cgit v1.2.3-58-ga151 From a19066788d875731a01ee7fa189b2202f0120036 Mon Sep 17 00:00:00 2001 From: Daniil Lunev Date: Thu, 28 Jul 2022 11:18:21 +1000 Subject: scsi: ufs: ufs-pci: Correct check for RESET DSM dsm_fns is a bitmap, and it is 0-indexed according to the check in __intel_dsm function. But common initialization was checking it as if it was 1-indexed. This patch corrects the discrepancy. This change won't break any existing calls to the function, since before the change both bits 0 and 1 were checked and needed to be set. Link: https://lore.kernel.org/r/20220728111748.v3.1.I22460c4f4a9ccf2c96c3f9bb392b409926d80b2f@changeid Reviewed-by: Adrian Hunter Signed-off-by: Daniil Lunev Signed-off-by: Martin K. Petersen --- drivers/ufs/host/ufshcd-pci.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/ufs/host/ufshcd-pci.c b/drivers/ufs/host/ufshcd-pci.c index 24af1f389bf2..1c91f43e15c8 100644 --- a/drivers/ufs/host/ufshcd-pci.c +++ b/drivers/ufs/host/ufshcd-pci.c @@ -24,7 +24,7 @@ struct ufs_host { void (*late_init)(struct ufs_hba *hba); }; -enum { +enum intel_ufs_dsm_func_id { INTEL_DSM_FNS = 0, INTEL_DSM_RESET = 1, }; @@ -42,6 +42,15 @@ static const guid_t intel_dsm_guid = GUID_INIT(0x1A4832A0, 0x7D03, 0x43CA, 0xB0, 0x20, 0xF6, 0xDC, 0xD1, 0x2A, 0x19, 0x50); +static bool __intel_dsm_supported(struct intel_host *host, + enum intel_ufs_dsm_func_id fn) +{ + return fn < 32 && fn >= 0 && (host->dsm_fns & (1u << fn)); +} + +#define INTEL_DSM_SUPPORTED(host, name) \ + __intel_dsm_supported(host, INTEL_DSM_##name) + static int __intel_dsm(struct intel_host *intel_host, struct device *dev, unsigned int fn, u32 *result) { @@ -71,7 +80,7 @@ out: static int intel_dsm(struct intel_host *intel_host, struct device *dev, unsigned int fn, u32 *result) { - if (fn > 31 || !(intel_host->dsm_fns & (1 << fn))) + if (!__intel_dsm_supported(intel_host, fn)) return -EOPNOTSUPP; return __intel_dsm(intel_host, dev, fn, result); @@ -300,7 +309,7 @@ static int ufs_intel_device_reset(struct ufs_hba *hba) { struct intel_host *host = ufshcd_get_variant(hba); - if (host->dsm_fns & INTEL_DSM_RESET) { + if (INTEL_DSM_SUPPORTED(host, RESET)) { u32 result = 0; int err; @@ -342,7 +351,7 @@ static int ufs_intel_common_init(struct ufs_hba *hba) return -ENOMEM; ufshcd_set_variant(hba, host); intel_dsm_init(host, hba->dev); - if (host->dsm_fns & INTEL_DSM_RESET) { + if (INTEL_DSM_SUPPORTED(host, RESET)) { if (hba->vops->device_reset) hba->caps |= UFSHCD_CAP_DEEPSLEEP; } else { -- cgit v1.2.3-58-ga151 From fe442604199ed3e60d5411137159f9623534e956 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 28 Jul 2022 15:18:48 -0700 Subject: scsi: core: Make sure that targets outlive devices This commit prevents that the following sequence triggers a kernel crash: - Deletion of a SCSI device is requested via sysfs. Device removal takes some time because blk_cleanup_queue() is waiting for the SCSI error handler. - The SCSI target associated with that SCSI device is removed. - scsi_remove_target() returns and its caller frees the resources associated with the SCSI target. - The error handler makes progress and invokes an LLD callback that dereferences the SCSI target pointer. Link: https://lore.kernel.org/r/20220728221851.1822295-2-bvanassche@acm.org Cc: Christoph Hellwig Cc: Mike Christie Cc: Hannes Reinecke Cc: John Garry Cc: Li Zhijian Reported-by: Mike Christie Reviewed-by: Ming Lei Reviewed-by: Mike Christie Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi_scan.c | 2 ++ drivers/scsi/scsi_sysfs.c | 20 +++++++++++++++++--- include/scsi/scsi_device.h | 2 ++ 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 91ac901a6682..4c1efd6a3b0c 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -521,6 +521,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, starget->state = STARGET_CREATED; starget->scsi_level = SCSI_2; starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED; + init_waitqueue_head(&starget->sdev_wq); + retry: spin_lock_irqsave(shost->host_lock, flags); diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 43949798a2e4..1bc9c26fe1d4 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -443,7 +443,9 @@ static void scsi_device_cls_release(struct device *class_dev) static void scsi_device_dev_release_usercontext(struct work_struct *work) { - struct scsi_device *sdev; + struct scsi_device *sdev = container_of(work, struct scsi_device, + ew.work); + struct scsi_target *starget = sdev->sdev_target; struct device *parent; struct list_head *this, *tmp; struct scsi_vpd *vpd_pg80 = NULL, *vpd_pg83 = NULL; @@ -452,8 +454,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) unsigned long flags; struct module *mod; - sdev = container_of(work, struct scsi_device, ew.work); - mod = sdev->host->hostt->module; scsi_dh_release_device(sdev); @@ -516,6 +516,9 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) kfree(sdev->inquiry); kfree(sdev); + if (starget && atomic_dec_return(&starget->sdev_count) == 0) + wake_up(&starget->sdev_wq); + if (parent) put_device(parent); module_put(mod); @@ -1535,6 +1538,14 @@ static void __scsi_remove_target(struct scsi_target *starget) goto restart; } spin_unlock_irqrestore(shost->host_lock, flags); + + /* + * After scsi_remove_target() returns its caller can remove resources + * associated with @starget, e.g. an rport or session. Wait until all + * devices associated with @starget have been removed to prevent that + * a SCSI error handling callback function triggers a use-after-free. + */ + wait_event(starget->sdev_wq, atomic_read(&starget->sdev_count) == 0); } /** @@ -1645,6 +1656,9 @@ void scsi_sysfs_device_initialize(struct scsi_device *sdev) list_add_tail(&sdev->same_target_siblings, &starget->devices); list_add_tail(&sdev->siblings, &shost->__devices); spin_unlock_irqrestore(shost->host_lock, flags); + + atomic_inc(&starget->sdev_count); + /* * device can now only be removed via __scsi_remove_device() so hold * the target. Target will be held in CREATED state until something diff --git a/include/scsi/scsi_device.h b/include/scsi/scsi_device.h index 7cf5f3b7589f..190d2081f4c6 100644 --- a/include/scsi/scsi_device.h +++ b/include/scsi/scsi_device.h @@ -309,6 +309,8 @@ struct scsi_target { struct list_head devices; struct device dev; struct kref reap_ref; /* last put renders target invisible */ + atomic_t sdev_count; + wait_queue_head_t sdev_wq; unsigned int channel; unsigned int id; /* target id ... replace * scsi_device.id eventually */ -- cgit v1.2.3-58-ga151 From 16728aaba62e8b3b170735fdc3d8aa972835c136 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 28 Jul 2022 15:18:49 -0700 Subject: scsi: core: Make sure that hosts outlive targets Fix the race conditions between SCSI LLD kernel module unloading and SCSI device and target removal by making sure that SCSI hosts are destroyed after all associated target and device objects have been freed. Link: https://lore.kernel.org/r/20220728221851.1822295-3-bvanassche@acm.org Cc: Christoph Hellwig Cc: Ming Lei Cc: Mike Christie Cc: Hannes Reinecke Cc: John Garry Reviewed-by: Mike Christie Signed-off-by: Ming Lei Signed-off-by: Bart Van Assche [ bvanassche: Reworked Ming's patch and split it ] Signed-off-by: Martin K. Petersen --- drivers/scsi/hosts.c | 8 ++++++++ drivers/scsi/scsi_scan.c | 7 +++++++ include/scsi/scsi_host.h | 3 +++ 3 files changed, 18 insertions(+) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index ef6c0e37acce..8fa98c8d0ee0 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -190,6 +190,13 @@ void scsi_remove_host(struct Scsi_Host *shost) transport_unregister_device(&shost->shost_gendev); device_unregister(&shost->shost_dev); device_del(&shost->shost_gendev); + + /* + * After scsi_remove_host() has returned the scsi LLD module can be + * unloaded and/or the host resources can be released. Hence wait until + * the dependent SCSI targets and devices are gone before returning. + */ + wait_event(shost->targets_wq, atomic_read(&shost->target_count) == 0); } EXPORT_SYMBOL(scsi_remove_host); @@ -394,6 +401,7 @@ struct Scsi_Host *scsi_host_alloc(struct scsi_host_template *sht, int privsize) INIT_LIST_HEAD(&shost->starved_list); init_waitqueue_head(&shost->host_wait); mutex_init(&shost->scan_mutex); + init_waitqueue_head(&shost->targets_wq); index = ida_alloc(&host_index_ida, GFP_KERNEL); if (index < 0) { diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c index 4c1efd6a3b0c..ac6059702d13 100644 --- a/drivers/scsi/scsi_scan.c +++ b/drivers/scsi/scsi_scan.c @@ -406,9 +406,14 @@ static void scsi_target_destroy(struct scsi_target *starget) static void scsi_target_dev_release(struct device *dev) { struct device *parent = dev->parent; + struct Scsi_Host *shost = dev_to_shost(parent); struct scsi_target *starget = to_scsi_target(dev); kfree(starget); + + if (atomic_dec_return(&shost->target_count) == 0) + wake_up(&shost->targets_wq); + put_device(parent); } @@ -523,6 +528,8 @@ static struct scsi_target *scsi_alloc_target(struct device *parent, starget->max_target_blocked = SCSI_DEFAULT_TARGET_BLOCKED; init_waitqueue_head(&starget->sdev_wq); + atomic_inc(&shost->target_count); + retry: spin_lock_irqsave(shost->host_lock, flags); diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h index 667d889b92b5..339f975d356e 100644 --- a/include/scsi/scsi_host.h +++ b/include/scsi/scsi_host.h @@ -689,6 +689,9 @@ struct Scsi_Host { /* ldm bits */ struct device shost_gendev, shost_dev; + atomic_t target_count; + wait_queue_head_t targets_wq; + /* * Points to the transport data (if any) which is allocated * separately -- cgit v1.2.3-58-ga151 From 1a9283782df2c91c7db5656753a2f548c43de6a7 Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 28 Jul 2022 15:18:50 -0700 Subject: scsi: core: Simplify LLD module reference counting Swap two statements in scsi_device_put() now that it is guaranteed that SCSI hosts outlive SCSI devices. Remove the reference counting code from scsi_sysfs.c that became superfluous because SCSI hosts now outlive SCSI devices. Link: https://lore.kernel.org/r/20220728221851.1822295-4-bvanassche@acm.org Cc: Christoph Hellwig Cc: Ming Lei Cc: Mike Christie Cc: Hannes Reinecke Cc: John Garry Reviewed-by: Mike Christie Signed-off-by: Ming Lei Signed-off-by: Bart Van Assche [ bvanassche: Extracted this patch from a larger patch ] Signed-off-by: Martin K. Petersen --- drivers/scsi/scsi.c | 9 ++++++--- drivers/scsi/scsi_sysfs.c | 9 --------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c index c59eac7a32f2..086ec5b5862d 100644 --- a/drivers/scsi/scsi.c +++ b/drivers/scsi/scsi.c @@ -586,10 +586,13 @@ EXPORT_SYMBOL(scsi_device_get); */ void scsi_device_put(struct scsi_device *sdev) { - struct module *mod = sdev->host->hostt->module; - + /* + * Decreasing the module reference count before the device reference + * count is safe since scsi_remove_host() only returns after all + * devices have been removed. + */ + module_put(sdev->host->hostt->module); put_device(&sdev->sdev_gendev); - module_put(mod); } EXPORT_SYMBOL(scsi_device_put); diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c index 1bc9c26fe1d4..213ebc88f76a 100644 --- a/drivers/scsi/scsi_sysfs.c +++ b/drivers/scsi/scsi_sysfs.c @@ -452,9 +452,6 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) struct scsi_vpd *vpd_pg0 = NULL, *vpd_pg89 = NULL; struct scsi_vpd *vpd_pgb0 = NULL, *vpd_pgb1 = NULL, *vpd_pgb2 = NULL; unsigned long flags; - struct module *mod; - - mod = sdev->host->hostt->module; scsi_dh_release_device(sdev); @@ -521,17 +518,11 @@ static void scsi_device_dev_release_usercontext(struct work_struct *work) if (parent) put_device(parent); - module_put(mod); } static void scsi_device_dev_release(struct device *dev) { struct scsi_device *sdp = to_scsi_device(dev); - - /* Set module pointer as NULL in case of module unloading */ - if (!try_module_get(sdp->host->hostt->module)) - sdp->host->hostt->module = NULL; - execute_in_process_context(scsi_device_dev_release_usercontext, &sdp->ew); } -- cgit v1.2.3-58-ga151 From f323896fe6fa8fa55ed37cb8b804fa97ead641c3 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Thu, 28 Jul 2022 15:18:51 -0700 Subject: scsi: core: Call blk_mq_free_tag_set() earlier There are two .exit_cmd_priv implementations. Both implementations use resources associated with the SCSI host. Make sure that these resources are still available when .exit_cmd_priv is called by moving the .exit_cmd_priv calls from scsi_host_dev_release() to scsi_forget_host(). Moving blk_mq_free_tag_set() from scsi_host_dev_release() to scsi_remove_host() is safe because scsi_forget_host() waits until all SCSI devices associated with the host have been removed. Fix the following use-after-free: ================================================================== BUG: KASAN: use-after-free in srp_exit_cmd_priv+0x27/0xd0 [ib_srp] Read of size 8 at addr ffff888100337000 by task multipathd/16727 Call Trace: dump_stack_lvl+0x34/0x44 print_report.cold+0x5e/0x5db kasan_report+0xab/0x120 srp_exit_cmd_priv+0x27/0xd0 [ib_srp] scsi_mq_exit_request+0x4d/0x70 blk_mq_free_rqs+0x143/0x410 __blk_mq_free_map_and_rqs+0x6e/0x100 blk_mq_free_tag_set+0x2b/0x160 scsi_host_dev_release+0xf3/0x1a0 device_release+0x54/0xe0 kobject_put+0xa5/0x120 device_release+0x54/0xe0 kobject_put+0xa5/0x120 scsi_device_dev_release_usercontext+0x4c1/0x4e0 execute_in_process_context+0x23/0x90 device_release+0x54/0xe0 kobject_put+0xa5/0x120 scsi_disk_release+0x3f/0x50 device_release+0x54/0xe0 kobject_put+0xa5/0x120 disk_release+0x17f/0x1b0 device_release+0x54/0xe0 kobject_put+0xa5/0x120 dm_put_table_device+0xa3/0x160 [dm_mod] dm_put_device+0xd0/0x140 [dm_mod] free_priority_group+0xd8/0x110 [dm_multipath] free_multipath+0x94/0xe0 [dm_multipath] dm_table_destroy+0xa2/0x1e0 [dm_mod] __dm_destroy+0x196/0x350 [dm_mod] dev_remove+0x10c/0x160 [dm_mod] ctl_ioctl+0x2c2/0x590 [dm_mod] dm_ctl_ioctl+0x5/0x10 [dm_mod] __x64_sys_ioctl+0xb4/0xf0 dm_ctl_ioctl+0x5/0x10 [dm_mod] __x64_sys_ioctl+0xb4/0xf0 do_syscall_64+0x3b/0x90 entry_SYSCALL_64_after_hwframe+0x46/0xb0 Link: https://lore.kernel.org/r/20220728221851.1822295-5-bvanassche@acm.org Fixes: 65ca846a5314 ("scsi: core: Introduce {init,exit}_cmd_priv()") Cc: Christoph Hellwig Cc: Mike Christie Cc: Hannes Reinecke Cc: John Garry Cc: Li Zhijian Reported-by: Li Zhijian Tested-by: Li Zhijian Reviewed-by: Ming Lei Reviewed-by: Mike Christie Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen --- drivers/scsi/hosts.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/scsi/hosts.c b/drivers/scsi/hosts.c index 8fa98c8d0ee0..6c63672971f1 100644 --- a/drivers/scsi/hosts.c +++ b/drivers/scsi/hosts.c @@ -197,6 +197,8 @@ void scsi_remove_host(struct Scsi_Host *shost) * the dependent SCSI targets and devices are gone before returning. */ wait_event(shost->targets_wq, atomic_read(&shost->target_count) == 0); + + scsi_mq_destroy_tags(shost); } EXPORT_SYMBOL(scsi_remove_host); @@ -302,8 +304,8 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, return error; /* - * Any host allocation in this function will be freed in - * scsi_host_dev_release(). + * Any resources associated with the SCSI host in this function except + * the tag set will be freed by scsi_host_dev_release(). */ out_del_dev: device_del(&shost->shost_dev); @@ -319,6 +321,7 @@ int scsi_add_host_with_dma(struct Scsi_Host *shost, struct device *dev, pm_runtime_disable(&shost->shost_gendev); pm_runtime_set_suspended(&shost->shost_gendev); pm_runtime_put_noidle(&shost->shost_gendev); + scsi_mq_destroy_tags(shost); fail: return error; } @@ -352,9 +355,6 @@ static void scsi_host_dev_release(struct device *dev) kfree(dev_name(&shost->shost_dev)); } - if (shost->tag_set.tags) - scsi_mq_destroy_tags(shost); - kfree(shost->shost_data); ida_free(&host_index_ida, shost->host_no); -- cgit v1.2.3-58-ga151 From 4da8c5f76825269f28d6a89fa752934a4bcb6dfa Mon Sep 17 00:00:00 2001 From: Steffen Maier Date: Fri, 29 Jul 2022 18:25:29 +0200 Subject: scsi: zfcp: Fix missing auto port scan and thus missing target ports Case (1): The only waiter on wka_port->completion_wq is zfcp_fc_wka_port_get() trying to open a WKA port. As such it should only be woken up by WKA port *open* responses, not by WKA port close responses. Case (2): A close WKA port response coming in just after having sent a new open WKA port request and before blocking for the open response with wait_event() in zfcp_fc_wka_port_get() erroneously renders the wait_event a NOP because the close handler overwrites wka_port->status. Hence the wait_event condition is erroneously true and it does not enter blocking state. With non-negligible probability, the following time space sequence happens depending on timing without this fix: user process ERP thread zfcp work queue tasklet system work queue ============ ========== =============== ======= ================= $ echo 1 > online zfcp_ccw_set_online zfcp_ccw_activate zfcp_erp_adapter_reopen msleep scan backoff zfcp_erp_strategy | ... | zfcp_erp_action_cleanup | ... | queue delayed scan_work | queue ns_up_work | ns_up_work: | zfcp_fc_wka_port_get | open wka request | open response | GSPN FC-GS | RSPN FC-GS [NPIV-only] | zfcp_fc_wka_port_put | (--wka->refcount==0) | sched delayed wka->work | ~~~Case (1)~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ zfcp_erp_wait flush scan_work | wka->work: | wka->status=CLOSING | close wka request | scan_work: | zfcp_fc_wka_port_get | (wka->status==CLOSING) | wka->status=OPENING | open wka request | wait_event | | close response | | wka->status=OFFLINE | | wake_up /*WRONG*/ ~~~Case (2)~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | wka->work: | wka->status=CLOSING | close wka request zfcp_erp_wait flush scan_work | scan_work: | zfcp_fc_wka_port_get | (wka->status==CLOSING) | wka->status=OPENING | open wka request | close response | wka->status=OFFLINE | wake_up /*WRONG&NOP*/ | wait_event /*NOP*/ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ | (wka->status!=ONLINE) | return -EIO | return early open response wka->status=ONLINE wake_up /*NOP*/ So we erroneously end up with no automatic port scan. This is a big problem when it happens during boot. The timing is influenced by v3.19 commit 18f87a67e6d6 ("zfcp: auto port scan resiliency"). Fix it by fully mutually excluding zfcp_fc_wka_port_get() and zfcp_fc_wka_port_offline(). For that to work, we make the latter block until we got the response for a close WKA port. In order not to penalize the system workqueue, we move wka_port->work to our own adapter workqueue. Note that before v2.6.30 commit 828bc1212a68 ("[SCSI] zfcp: Set WKA-port to offline on adapter deactivation"), zfcp did block in zfcp_fc_wka_port_offline() as well, but with a different condition. While at it, make non-functional cleanups to improve code reading in zfcp_fc_wka_port_get(). If we cannot send the WKA port open request, don't rely on the subsequent wait_event condition to immediately let this case pass without blocking. Also don't want to rely on the additional condition handling the refcount to be skipped just to finally return with -EIO. Link: https://lore.kernel.org/r/20220729162529.1620730-1-maier@linux.ibm.com Fixes: 5ab944f97e09 ("[SCSI] zfcp: attach and release SAN nameserver port on demand") Cc: #v2.6.28+ Reviewed-by: Benjamin Block Signed-off-by: Steffen Maier Signed-off-by: Martin K. Petersen --- drivers/s390/scsi/zfcp_fc.c | 29 ++++++++++++++++++++--------- drivers/s390/scsi/zfcp_fc.h | 6 ++++-- drivers/s390/scsi/zfcp_fsf.c | 4 ++-- 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c index 511bf8e0a436..b61acbb09be3 100644 --- a/drivers/s390/scsi/zfcp_fc.c +++ b/drivers/s390/scsi/zfcp_fc.c @@ -145,27 +145,33 @@ void zfcp_fc_enqueue_event(struct zfcp_adapter *adapter, static int zfcp_fc_wka_port_get(struct zfcp_fc_wka_port *wka_port) { + int ret = -EIO; + if (mutex_lock_interruptible(&wka_port->mutex)) return -ERESTARTSYS; if (wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE || wka_port->status == ZFCP_FC_WKA_PORT_CLOSING) { wka_port->status = ZFCP_FC_WKA_PORT_OPENING; - if (zfcp_fsf_open_wka_port(wka_port)) + if (zfcp_fsf_open_wka_port(wka_port)) { + /* could not even send request, nothing to wait for */ wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE; + goto out; + } } - mutex_unlock(&wka_port->mutex); - - wait_event(wka_port->completion_wq, + wait_event(wka_port->opened, wka_port->status == ZFCP_FC_WKA_PORT_ONLINE || wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE); if (wka_port->status == ZFCP_FC_WKA_PORT_ONLINE) { atomic_inc(&wka_port->refcount); - return 0; + ret = 0; + goto out; } - return -EIO; +out: + mutex_unlock(&wka_port->mutex); + return ret; } static void zfcp_fc_wka_port_offline(struct work_struct *work) @@ -181,9 +187,12 @@ static void zfcp_fc_wka_port_offline(struct work_struct *work) wka_port->status = ZFCP_FC_WKA_PORT_CLOSING; if (zfcp_fsf_close_wka_port(wka_port)) { + /* could not even send request, nothing to wait for */ wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE; - wake_up(&wka_port->completion_wq); + goto out; } + wait_event(wka_port->closed, + wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE); out: mutex_unlock(&wka_port->mutex); } @@ -193,13 +202,15 @@ static void zfcp_fc_wka_port_put(struct zfcp_fc_wka_port *wka_port) if (atomic_dec_return(&wka_port->refcount) != 0) return; /* wait 10 milliseconds, other reqs might pop in */ - schedule_delayed_work(&wka_port->work, HZ / 100); + queue_delayed_work(wka_port->adapter->work_queue, &wka_port->work, + msecs_to_jiffies(10)); } static void zfcp_fc_wka_port_init(struct zfcp_fc_wka_port *wka_port, u32 d_id, struct zfcp_adapter *adapter) { - init_waitqueue_head(&wka_port->completion_wq); + init_waitqueue_head(&wka_port->opened); + init_waitqueue_head(&wka_port->closed); wka_port->adapter = adapter; wka_port->d_id = d_id; diff --git a/drivers/s390/scsi/zfcp_fc.h b/drivers/s390/scsi/zfcp_fc.h index 8aaf409ce9cb..97755407ce1b 100644 --- a/drivers/s390/scsi/zfcp_fc.h +++ b/drivers/s390/scsi/zfcp_fc.h @@ -185,7 +185,8 @@ enum zfcp_fc_wka_status { /** * struct zfcp_fc_wka_port - representation of well-known-address (WKA) FC port * @adapter: Pointer to adapter structure this WKA port belongs to - * @completion_wq: Wait for completion of open/close command + * @opened: Wait for completion of open command + * @closed: Wait for completion of close command * @status: Current status of WKA port * @refcount: Reference count to keep port open as long as it is in use * @d_id: FC destination id or well-known-address @@ -195,7 +196,8 @@ enum zfcp_fc_wka_status { */ struct zfcp_fc_wka_port { struct zfcp_adapter *adapter; - wait_queue_head_t completion_wq; + wait_queue_head_t opened; + wait_queue_head_t closed; enum zfcp_fc_wka_status status; atomic_t refcount; u32 d_id; diff --git a/drivers/s390/scsi/zfcp_fsf.c b/drivers/s390/scsi/zfcp_fsf.c index 4f1e4385ce58..19223b075568 100644 --- a/drivers/s390/scsi/zfcp_fsf.c +++ b/drivers/s390/scsi/zfcp_fsf.c @@ -1907,7 +1907,7 @@ static void zfcp_fsf_open_wka_port_handler(struct zfcp_fsf_req *req) wka_port->status = ZFCP_FC_WKA_PORT_ONLINE; } out: - wake_up(&wka_port->completion_wq); + wake_up(&wka_port->opened); } /** @@ -1966,7 +1966,7 @@ static void zfcp_fsf_close_wka_port_handler(struct zfcp_fsf_req *req) } wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE; - wake_up(&wka_port->completion_wq); + wake_up(&wka_port->closed); } /** -- cgit v1.2.3-58-ga151 From 554b117e8fab4f7c53090eca693f47e0c7b18490 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sat, 30 Jul 2022 13:37:36 +0100 Subject: scsi: FlashPoint: Remove redundant variable bm_int_st The variable bm_int_st is assigned a value but it is never read. The variable and the assignment are redundant and can be removed. Cleans up clang scan build warning: drivers/scsi/FlashPoint.c:1726:7: warning: Although the value stored to 'bm_int_st' is used in the enclosing expression, the value is never actually read from 'bm_int_st' [deadcode.DeadStores] Link: https://lore.kernel.org/r/20220730123736.147758-1-colin.i.king@gmail.com Acked-by: Khalid Aziz Signed-off-by: Colin Ian King Signed-off-by: Martin K. Petersen --- drivers/scsi/FlashPoint.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/FlashPoint.c b/drivers/scsi/FlashPoint.c index 90253208a72f..3d9c56ac8224 100644 --- a/drivers/scsi/FlashPoint.c +++ b/drivers/scsi/FlashPoint.c @@ -1712,7 +1712,7 @@ static unsigned char FlashPoint_InterruptPending(void *pCurrCard) static int FlashPoint_HandleInterrupt(void *pcard) { struct sccb *currSCCB; - unsigned char thisCard, result, bm_status, bm_int_st; + unsigned char thisCard, result, bm_status; unsigned short hp_int; unsigned char i, target; struct sccb_card *pCurrCard = pcard; @@ -1723,7 +1723,7 @@ static int FlashPoint_HandleInterrupt(void *pcard) MDISABLE_INT(ioport); - if ((bm_int_st = RD_HARPOON(ioport + hp_int_status)) & EXT_STATUS_ON) + if (RD_HARPOON(ioport + hp_int_status) & EXT_STATUS_ON) bm_status = RD_HARPOON(ioport + hp_ext_status) & (unsigned char)BAD_EXT_STATUS; else -- cgit v1.2.3-58-ga151 From 6464d5b8a2768e8ff63d24b76299fe614e205aa7 Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Sat, 30 Jul 2022 13:45:09 +0100 Subject: scsi: megaraid_sas: Remove redundant variable cmd_type The variable cmd_type is assigned a value but it is never read. The variable and the assignment are redundant and can be removed. Cleans up clang scan build warning: drivers/scsi/megaraid/megaraid_sas_fusion.c:3228:10: warning: Although the value stored to 'cmd_type' is used in the enclosing expression, the value is never actually read from 'cmd_type' [deadcode.DeadStores] Link: https://lore.kernel.org/r/20220730124509.148457-1-colin.i.king@gmail.com Signed-off-by: Colin Ian King Signed-off-by: Martin K. Petersen --- drivers/scsi/megaraid/megaraid_sas_fusion.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index 5b5885d9732b..e48d4261d0bc 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -3199,7 +3199,6 @@ megasas_build_io_fusion(struct megasas_instance *instance, struct megasas_cmd_fusion *cmd) { int sge_count; - u8 cmd_type; u16 pd_index = 0; u8 drive_type = 0; struct MPI2_RAID_SCSI_IO_REQUEST *io_request = cmd->io_request; @@ -3225,7 +3224,7 @@ megasas_build_io_fusion(struct megasas_instance *instance, */ io_request->IoFlags = cpu_to_le16(scp->cmd_len); - switch (cmd_type = megasas_cmd_type(scp)) { + switch (megasas_cmd_type(scp)) { case READ_WRITE_LDIO: megasas_build_ldio_fusion(instance, scp, cmd); break; -- cgit v1.2.3-58-ga151 From c6380f9924270d51cc233cfd592b279be3881e6d Mon Sep 17 00:00:00 2001 From: Slark Xiao Date: Fri, 22 Jul 2022 17:46:12 +0800 Subject: scsi: pm8001: Fix typo 'the the' in comment Replace 'the the' with 'the' in the comment. Link: https://lore.kernel.org/r/20220722094612.78583-1-slark_xiao@163.com Acked-by: Jack Wang Signed-off-by: Slark Xiao Signed-off-by: Martin K. Petersen --- drivers/scsi/pm8001/pm8001_hwi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index a37595621d51..6a3c4c347061 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -3138,7 +3138,7 @@ int pm8001_mpi_local_phy_ctl(struct pm8001_hba_info *pm8001_ha, void *piomb) * * when HBA driver received the identify done event or initiate FIS received * event(for SATA), it will invoke this function to notify the sas layer that - * the sas toplogy has formed, please discover the the whole sas domain, + * the sas toplogy has formed, please discover the whole sas domain, * while receive a broadcast(change) primitive just tell the sas * layer to discover the changed domain rather than the whole domain. */ -- cgit v1.2.3-58-ga151