From patchwork Wed Jun 17 08:35:12 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 11609449 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id AF90814DD for ; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A19482100A for ; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726909AbgFQIf5 (ORCPT ); Wed, 17 Jun 2020 04:35:57 -0400 Received: from mx2.suse.de ([195.135.220.15]:52110 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726879AbgFQIfz (ORCPT ); Wed, 17 Jun 2020 04:35:55 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 891A7AD5E; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) From: mwilck@suse.com To: Don Brace , "Martin K. Petersen" Cc: esc.storagedev@microsemi.com, linux-scsi@vger.kernel.org, shane.seymour@hpe.com, Martin Wilck Subject: [PATCH v2 1/3] scsi: smartpqi: grab scsi device ref in slave_configure() Date: Wed, 17 Jun 2020 10:35:12 +0200 Message-Id: <20200617083514.19174-2-mwilck@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200617083514.19174-1-mwilck@suse.com> References: <20200617083514.19174-1-mwilck@suse.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: Martin Wilck We have observed kernel crashes caused by the smartpqi driver holding a pointer to a struct scsi_device that had been removed already. Fix this by getting and holding a ref for the device as long as the driver uses it. Use a lock to avoid races between device probing and removal. Reviewed-by: Shane Seymour Signed-off-by: Martin Wilck --- drivers/scsi/smartpqi/smartpqi.h | 1 + drivers/scsi/smartpqi/smartpqi_init.c | 122 +++++++++++++++++++++----- 2 files changed, 103 insertions(+), 20 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h index 1129fe7a27ed..5801080c8dbc 100644 --- a/drivers/scsi/smartpqi/smartpqi.h +++ b/drivers/scsi/smartpqi/smartpqi.h @@ -954,6 +954,7 @@ struct pqi_scsi_dev { struct raid_map *raid_map; /* RAID bypass map */ struct pqi_sas_port *sas_port; + spinlock_t sdev_lock; /* protect access to sdev */ struct scsi_device *sdev; struct list_head scsi_device_list_entry; diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index cd157f11eb22..54a72f465f85 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1514,6 +1514,18 @@ static int pqi_add_device(struct pqi_ctrl_info *ctrl_info, #define PQI_PENDING_IO_TIMEOUT_SECS 20 +static inline struct scsi_device * +pqi_get_scsi_device(struct pqi_scsi_dev *device) +{ + unsigned long flags; + struct scsi_device *sdev; + + spin_lock_irqsave(&device->sdev_lock, flags); + sdev = device->sdev; + spin_unlock_irqrestore(&device->sdev_lock, flags); + return sdev; +} + static inline void pqi_remove_device(struct pqi_ctrl_info *ctrl_info, struct pqi_scsi_dev *device) { @@ -1530,9 +1542,26 @@ static inline void pqi_remove_device(struct pqi_ctrl_info *ctrl_info, device->target, device->lun, atomic_read(&device->scsi_cmds_outstanding)); - if (pqi_is_logical_device(device)) - scsi_remove_device(device->sdev); - else + if (pqi_is_logical_device(device)) { + struct scsi_device *sdev; + unsigned long flags; + + spin_lock_irqsave(&device->sdev_lock, flags); + sdev = device->sdev; + if (sdev) + get_device(&sdev->sdev_gendev); + spin_unlock_irqrestore(&device->sdev_lock, flags); + + /* + * As scsi_remove_device() will call pqi_slave_destroy(), + * we can't keep the sdev_lock held. But we've taken a ref, + * so sdev will not go away under us. + */ + if (sdev) { + scsi_remove_device(sdev); + put_device(&sdev->sdev_gendev); + } + } else pqi_remove_sas_device(device); } @@ -1749,7 +1778,7 @@ static inline bool pqi_is_device_added(struct pqi_scsi_dev *device) if (device->is_expander_smp_device) return device->sas_port != NULL; - return device->sdev != NULL; + return pqi_get_scsi_device(device) != NULL; } static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info, @@ -1861,11 +1890,24 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info, */ list_for_each_entry(device, &ctrl_info->scsi_device_list, scsi_device_list_entry) { - if (device->sdev && device->queue_depth != - device->advertised_queue_depth) { - device->advertised_queue_depth = device->queue_depth; - scsi_change_queue_depth(device->sdev, - device->advertised_queue_depth); + struct scsi_device *sdev; + unsigned long flags; + + spin_lock_irqsave(&device->sdev_lock, flags); + sdev = device->sdev; + if (sdev) + get_device(&sdev->sdev_gendev); + spin_unlock_irqrestore(&device->sdev_lock, flags); + + if (sdev) { + if (device->queue_depth != + device->advertised_queue_depth) { + device->advertised_queue_depth = + device->queue_depth; + scsi_change_queue_depth(sdev, + device->advertised_queue_depth); + } + put_device(&sdev->sdev_gendev); } } @@ -2082,6 +2124,7 @@ static int pqi_update_scsi_devices(struct pqi_ctrl_info *ctrl_info) device = list_first_entry(&new_device_list_head, struct pqi_scsi_dev, new_device_list_entry); + spin_lock_init(&device->sdev_lock); memcpy(device->scsi3addr, scsi3addr, sizeof(device->scsi3addr)); device->is_physical_device = is_physical_device; if (is_physical_device) { @@ -5785,6 +5828,7 @@ static int pqi_slave_alloc(struct scsi_device *sdev) struct pqi_ctrl_info *ctrl_info; struct scsi_target *starget; struct sas_rphy *rphy; + int ret; ctrl_info = shost_to_hba(sdev->host); @@ -5806,23 +5850,59 @@ static int pqi_slave_alloc(struct scsi_device *sdev) if (device) { sdev->hostdata = device; - device->sdev = sdev; - if (device->queue_depth) { - device->advertised_queue_depth = device->queue_depth; - scsi_change_queue_depth(sdev, - device->advertised_queue_depth); - } - if (pqi_is_logical_device(device)) - pqi_disable_write_same(sdev); - else - sdev->allow_restart = 1; - } + ret = 0; + } else + ret = -ENXIO; spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags); + return ret; +} + +static int pqi_slave_configure(struct scsi_device *sdev) +{ + struct pqi_scsi_dev *device = sdev->hostdata; + unsigned long flags; + + /* + * Grab a ref to the SCSI device, lest it be removed under us. It will + * be dropped in pqi_slave_destroy(). + * Don't use scsi_device_get() here, we'd increment our own use count + */ + if (!get_device(&sdev->sdev_gendev)) + return -ENXIO; + + spin_lock_irqsave(&device->sdev_lock, flags); + device->sdev = sdev; + spin_unlock_irqrestore(&device->sdev_lock, flags); + + if (device->queue_depth) { + device->advertised_queue_depth = device->queue_depth; + scsi_change_queue_depth(sdev, + device->advertised_queue_depth); + } + if (pqi_is_logical_device(device)) + pqi_disable_write_same(sdev); + else + sdev->allow_restart = 1; return 0; } +static void pqi_slave_destroy(struct scsi_device *sdev) +{ + struct pqi_scsi_dev *device = sdev->hostdata; + unsigned long flags; + + if (!device) + return; + + spin_lock_irqsave(&device->sdev_lock, flags); + sdev = device->sdev; + device->sdev = NULL; + spin_unlock_irqrestore(&device->sdev_lock, flags); + put_device(&sdev->sdev_gendev); +} + static int pqi_map_queues(struct Scsi_Host *shost) { struct pqi_ctrl_info *ctrl_info = shost_to_hba(shost); @@ -6501,6 +6581,8 @@ static struct scsi_host_template pqi_driver_template = { .eh_device_reset_handler = pqi_eh_device_reset_handler, .ioctl = pqi_ioctl, .slave_alloc = pqi_slave_alloc, + .slave_configure = pqi_slave_configure, + .slave_destroy = pqi_slave_destroy, .map_queues = pqi_map_queues, .sdev_attrs = pqi_sdev_attrs, .shost_attrs = pqi_shost_attrs, From patchwork Wed Jun 17 08:35:13 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 11609445 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id EE62290 for ; Wed, 17 Jun 2020 08:35:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id DE71B21527 for ; Wed, 17 Jun 2020 08:35:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726044AbgFQIf4 (ORCPT ); Wed, 17 Jun 2020 04:35:56 -0400 Received: from mx2.suse.de ([195.135.220.15]:52104 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726868AbgFQIfz (ORCPT ); Wed, 17 Jun 2020 04:35:55 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 89269AE0E; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) From: mwilck@suse.com To: Don Brace , "Martin K. Petersen" Cc: esc.storagedev@microsemi.com, linux-scsi@vger.kernel.org, shane.seymour@hpe.com, Martin Wilck Subject: [PATCH v2 2/3] scsi: smartpqi: check sdev in pqi_scsi_find_entry Date: Wed, 17 Jun 2020 10:35:13 +0200 Message-Id: <20200617083514.19174-3-mwilck@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200617083514.19174-1-mwilck@suse.com> References: <20200617083514.19174-1-mwilck@suse.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: Martin Wilck If a scsi device has been destroyed e.g. using the sysfs "delete" attribute, subsequent host rescans won't re-discover it. This patch makes it work at least via the smartqpi-specific "rescan" sysfs attribute. Reviewed-by: Shane Seymour Signed-off-by: Martin Wilck --- drivers/scsi/smartpqi/smartpqi_init.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 54a72f465f85..87089b67ff74 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1612,7 +1612,8 @@ static enum pqi_find_result pqi_scsi_find_entry(struct pqi_ctrl_info *ctrl_info, device->scsi3addr)) { *matching_device = device; if (pqi_device_equal(device_to_find, device)) { - if (device_to_find->volume_offline) + if (device_to_find->volume_offline || + !pqi_get_scsi_device(device)) return DEVICE_CHANGED; return DEVICE_SAME; } From patchwork Wed Jun 17 08:35:14 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Wilck X-Patchwork-Id: 11609447 Return-Path: Received: from mail.kernel.org (pdx-korg-mail-1.web.codeaurora.org [172.30.200.123]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 7BCA71746 for ; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 6D965212CC for ; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726905AbgFQIf4 (ORCPT ); Wed, 17 Jun 2020 04:35:56 -0400 Received: from mx2.suse.de ([195.135.220.15]:52108 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726519AbgFQIfz (ORCPT ); Wed, 17 Jun 2020 04:35:55 -0400 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id A5381AC46; Wed, 17 Jun 2020 08:35:57 +0000 (UTC) From: mwilck@suse.com To: Don Brace , "Martin K. Petersen" Cc: esc.storagedev@microsemi.com, linux-scsi@vger.kernel.org, shane.seymour@hpe.com, Martin Wilck Subject: [PATCH v2 3/3] scsi: smartpqi: remove conditional before pqi_remove_device() Date: Wed, 17 Jun 2020 10:35:14 +0200 Message-Id: <20200617083514.19174-4-mwilck@suse.com> X-Mailer: git-send-email 2.26.2 In-Reply-To: <20200617083514.19174-1-mwilck@suse.com> References: <20200617083514.19174-1-mwilck@suse.com> MIME-Version: 1.0 Sender: linux-scsi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-scsi@vger.kernel.org From: Martin Wilck pqi_remove_device() checks if there's anything to remove, for both logical and SAS devices. So these conditionals are redundant. They may actually be wrong, because they would skip removing pysical devices which are not SMP expanders. Signed-off-by: Martin Wilck Reviewed-by: Shane Seymour --- drivers/scsi/smartpqi/smartpqi_init.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c index 87089b67ff74..7e4d5c5ea2b0 100644 --- a/drivers/scsi/smartpqi/smartpqi_init.c +++ b/drivers/scsi/smartpqi/smartpqi_init.c @@ -1879,8 +1879,7 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info, } else { pqi_dev_info(ctrl_info, "removed", device); } - if (pqi_is_device_added(device)) - pqi_remove_device(ctrl_info, device); + pqi_remove_device(ctrl_info, device); list_del(&device->delete_list_entry); pqi_free_device(device); } @@ -2223,8 +2222,7 @@ static void pqi_remove_all_scsi_devices(struct pqi_ctrl_info *ctrl_info) if (!device) break; - if (pqi_is_device_added(device)) - pqi_remove_device(ctrl_info, device); + pqi_remove_device(ctrl_info, device); pqi_free_device(device); } }