From: Sasha Levin <sashal@kernel.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Mahesh Rajashekhara <mahesh.rajashekhara@microchip.com>,
Scott Teel <scott.teel@microchip.com>,
Scott Benesh <scott.benesh@microchip.com>,
Mike McGowen <mike.mcgowen@microchip.com>,
Kevin Barnett <kevin.barnett@microchip.com>,
Murthy Bhat <Murthy.Bhat@microchip.com>,
Don Brace <don.brace@microchip.com>,
"Martin K . Petersen" <martin.petersen@oracle.com>,
Sasha Levin <sashal@kernel.org>,
jejb@linux.ibm.com, storagedev@microchip.com,
linux-scsi@vger.kernel.org
Subject: [PATCH AUTOSEL 6.6 03/10] scsi: smartpqi: Fix logical volume rescan race condition
Date: Mon, 29 Jan 2024 13:35:11 -0500 [thread overview]
Message-ID: <20240129183530.464274-3-sashal@kernel.org> (raw)
In-Reply-To: <20240129183530.464274-1-sashal@kernel.org>
From: Mahesh Rajashekhara <mahesh.rajashekhara@microchip.com>
[ Upstream commit fb4cece17b4583f55b34a8538e27a4adc833c9d4 ]
Correct rescan flag race condition.
Multiple conditions are being evaluated before notifying OS to do a rescan.
Driver will skip rescanning the device if any one of the following
conditions are met:
- Devices that have not yet been added to the OS or devices that have been
removed.
- Devices which are already marked for removal or in the phase of removal.
Under very rare conditions, after logical volume size expansion, the OS
still sees the size of the logical volume which was before expansion.
The rescan flag in the driver is used to signal the need for a logical
volume rescan. A race condition can occur in the driver, and it leads to
one thread overwriting the flag inadvertently. As a result, driver is not
notifying the OS SML to rescan the logical volume.
Move device->rescan update into new function pqi_mark_volumes_for_rescan()
and protect with a spin lock.
Move check for device->rescan into new function pqi_volume_rescan_needed()
and protect function call with a spin_lock.
Reviewed-by: Scott Teel <scott.teel@microchip.com>
Reviewed-by: Scott Benesh <scott.benesh@microchip.com>
Reviewed-by: Mike McGowen <mike.mcgowen@microchip.com>
Reviewed-by: Kevin Barnett <kevin.barnett@microchip.com>
Co-developed-by: Murthy Bhat <Murthy.Bhat@microchip.com>
Signed-off-by: Murthy Bhat <Murthy.Bhat@microchip.com>
Signed-off-by: Mahesh Rajashekhara <mahesh.rajashekhara@microchip.com>
Signed-off-by: Don Brace <don.brace@microchip.com>
Link: https://lore.kernel.org/r/20231219193653.277553-3-don.brace@microchip.com
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/scsi/smartpqi/smartpqi.h | 1 -
drivers/scsi/smartpqi/smartpqi_init.c | 43 ++++++++++++++++++++++-----
2 files changed, 36 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi.h b/drivers/scsi/smartpqi/smartpqi.h
index 041940183516..cdedc271857a 100644
--- a/drivers/scsi/smartpqi/smartpqi.h
+++ b/drivers/scsi/smartpqi/smartpqi.h
@@ -1347,7 +1347,6 @@ struct pqi_ctrl_info {
bool controller_online;
bool block_requests;
bool scan_blocked;
- u8 logical_volume_rescan_needed : 1;
u8 inbound_spanning_supported : 1;
u8 outbound_spanning_supported : 1;
u8 pqi_mode_enabled : 1;
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c b/drivers/scsi/smartpqi/smartpqi_init.c
index d56201120087..081bb2c09806 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -2093,8 +2093,6 @@ static void pqi_scsi_update_device(struct pqi_ctrl_info *ctrl_info,
if (existing_device->devtype == TYPE_DISK) {
existing_device->raid_level = new_device->raid_level;
existing_device->volume_status = new_device->volume_status;
- if (ctrl_info->logical_volume_rescan_needed)
- existing_device->rescan = true;
memset(existing_device->next_bypass_group, 0, sizeof(existing_device->next_bypass_group));
if (!pqi_raid_maps_equal(existing_device->raid_map, new_device->raid_map)) {
kfree(existing_device->raid_map);
@@ -2164,6 +2162,20 @@ static inline void pqi_init_device_tmf_work(struct pqi_scsi_dev *device)
INIT_WORK(&tmf_work->work_struct, pqi_tmf_worker);
}
+static inline bool pqi_volume_rescan_needed(struct pqi_scsi_dev *device)
+{
+ if (pqi_device_in_remove(device))
+ return false;
+
+ if (device->sdev == NULL)
+ return false;
+
+ if (!scsi_device_online(device->sdev))
+ return false;
+
+ return device->rescan;
+}
+
static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info,
struct pqi_scsi_dev *new_device_list[], unsigned int num_new_devices)
{
@@ -2284,9 +2296,13 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info,
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);
- if (device->rescan) {
- scsi_rescan_device(device->sdev);
+ spin_lock_irqsave(&ctrl_info->scsi_device_list_lock, flags);
+ if (pqi_volume_rescan_needed(device)) {
device->rescan = false;
+ spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags);
+ scsi_rescan_device(device->sdev);
+ } else {
+ spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags);
}
}
}
@@ -2308,8 +2324,6 @@ static void pqi_update_device_list(struct pqi_ctrl_info *ctrl_info,
}
}
- ctrl_info->logical_volume_rescan_needed = false;
-
}
static inline bool pqi_is_supported_device(struct pqi_scsi_dev *device)
@@ -3702,6 +3716,21 @@ static bool pqi_ofa_process_event(struct pqi_ctrl_info *ctrl_info,
return ack_event;
}
+static void pqi_mark_volumes_for_rescan(struct pqi_ctrl_info *ctrl_info)
+{
+ unsigned long flags;
+ struct pqi_scsi_dev *device;
+
+ spin_lock_irqsave(&ctrl_info->scsi_device_list_lock, flags);
+
+ list_for_each_entry(device, &ctrl_info->scsi_device_list, scsi_device_list_entry) {
+ if (pqi_is_logical_device(device) && device->devtype == TYPE_DISK)
+ device->rescan = true;
+ }
+
+ spin_unlock_irqrestore(&ctrl_info->scsi_device_list_lock, flags);
+}
+
static void pqi_disable_raid_bypass(struct pqi_ctrl_info *ctrl_info)
{
unsigned long flags;
@@ -3742,7 +3771,7 @@ static void pqi_event_worker(struct work_struct *work)
ack_event = true;
rescan_needed = true;
if (event->event_type == PQI_EVENT_TYPE_LOGICAL_DEVICE)
- ctrl_info->logical_volume_rescan_needed = true;
+ pqi_mark_volumes_for_rescan(ctrl_info);
else if (event->event_type == PQI_EVENT_TYPE_AIO_STATE_CHANGE)
pqi_disable_raid_bypass(ctrl_info);
}
--
2.43.0
next prev parent reply other threads:[~2024-01-29 18:35 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-29 18:35 [PATCH AUTOSEL 6.6 01/10] dmaengine: apple-admac: Keep upper bits of REG_BUS_WIDTH Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 02/10] scsi: smartpqi: Add new controller PCI IDs Sasha Levin
2024-01-29 18:35 ` Sasha Levin [this message]
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 04/10] tools: selftests: riscv: Fix compile warnings in vector tests Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 05/10] tools: selftests: riscv: Fix compile warnings in mm tests Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 06/10] scsi: target: core: Add TMF to tmr_list handling Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 07/10] cifs: open_cached_dir should not rely on primary channel Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 08/10] dmaengine: shdma: increase size of 'dev_id' Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 09/10] dmaengine: fsl-qdma: increase size of 'irq_name' Sasha Levin
2024-01-29 18:35 ` [PATCH AUTOSEL 6.6 10/10] dmaengine: dw-edma: increase size of 'name' in debugfs code Sasha Levin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240129183530.464274-3-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=Murthy.Bhat@microchip.com \
--cc=don.brace@microchip.com \
--cc=jejb@linux.ibm.com \
--cc=kevin.barnett@microchip.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=mahesh.rajashekhara@microchip.com \
--cc=martin.petersen@oracle.com \
--cc=mike.mcgowen@microchip.com \
--cc=scott.benesh@microchip.com \
--cc=scott.teel@microchip.com \
--cc=stable@vger.kernel.org \
--cc=storagedev@microchip.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox