Skip to content

Commit ef4021f

Browse files
steffen-maiermartinkpetersen
authored andcommitted
scsi: zfcp: fix to prevent port_remove with pure auto scan LUNs (only sdevs)
When the user tries to remove a zfcp port via sysfs, we only rejected it if there are zfcp unit children under the port. With purely automatically scanned LUNs there are no zfcp units but only SCSI devices. In such cases, the port_remove erroneously continued. We close the port and this implicitly closes all LUNs under the port. The SCSI devices survive with their private zfcp_scsi_dev still holding a reference to the "removed" zfcp_port (still allocated but invisible in sysfs) [zfcp_get_port_by_wwpn in zfcp_scsi_slave_alloc]. This is not a problem as long as the fc_rport stays blocked. Once (auto) port scan brings back the removed port, we unblock its fc_rport again by design. However, there is no mechanism that would recover (open) the LUNs under the port (no "ersfs_3" without zfcp_unit [zfcp_erp_strategy_followup_success]). Any pending or new I/O to such LUN leads to repeated: Done: NEEDS_RETRY Result: hostbyte=DID_IMM_RETRY driverbyte=DRIVER_OK See also v4.10 commit 6f2ce1c ("scsi: zfcp: fix rport unblock race with LUN recovery"). Even a manual LUN recovery (echo 0 > /sys/bus/scsi/devices/H:C:T:L/zfcp_failed) does not help, as the LUN links to the old "removed" port which remains to lack ZFCP_STATUS_COMMON_RUNNING [zfcp_erp_required_act]. The only workaround is to first ensure that the fc_rport is blocked (e.g. port_remove again in case it was re-discovered by (auto) port scan), then delete the SCSI devices, and finally re-discover by (auto) port scan. The port scan includes an fc_rport unblock, which in turn triggers a new scan on the scsi target to freshly get new pure auto scan LUNs. Fix this by rejecting port_remove also if there are SCSI devices (even without any zfcp_unit) under this port. Re-use mechanics from v3.7 commit d99b601 ("[SCSI] zfcp: restore refcount check on port_remove"). However, we have to give up zfcp_sysfs_port_units_mutex earlier in unit_add to prevent a deadlock with scsi_host scan taking shost->scan_mutex first and then zfcp_sysfs_port_units_mutex now in our zfcp_scsi_slave_alloc(). Signed-off-by: Steffen Maier <maier@linux.ibm.com> Fixes: b62a8d9 ("[SCSI] zfcp: Use SCSI device data zfcp scsi dev instead of zfcp unit") Fixes: f8210e3 ("[SCSI] zfcp: Allow midlayer to scan for LUNs when running in NPIV mode") Cc: <stable@vger.kernel.org> #2.6.37+ Reviewed-by: Benjamin Block <bblock@linux.ibm.com> Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
1 parent d27e5e0 commit ef4021f

File tree

4 files changed

+65
-7
lines changed

4 files changed

+65
-7
lines changed

drivers/s390/scsi/zfcp_ext.h

+1
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,7 @@ extern const struct attribute_group *zfcp_port_attr_groups[];
167167
extern struct mutex zfcp_sysfs_port_units_mutex;
168168
extern struct device_attribute *zfcp_sysfs_sdev_attrs[];
169169
extern struct device_attribute *zfcp_sysfs_shost_attrs[];
170+
bool zfcp_sysfs_port_is_removing(const struct zfcp_port *const port);
170171

171172
/* zfcp_unit.c */
172173
extern int zfcp_unit_add(struct zfcp_port *, u64);

drivers/s390/scsi/zfcp_scsi.c

+9
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,15 @@ static int zfcp_scsi_slave_alloc(struct scsi_device *sdev)
129129

130130
zfcp_sdev->erp_action.port = port;
131131

132+
mutex_lock(&zfcp_sysfs_port_units_mutex);
133+
if (zfcp_sysfs_port_is_removing(port)) {
134+
/* port is already gone */
135+
mutex_unlock(&zfcp_sysfs_port_units_mutex);
136+
put_device(&port->dev); /* undo zfcp_get_port_by_wwpn() */
137+
return -ENXIO;
138+
}
139+
mutex_unlock(&zfcp_sysfs_port_units_mutex);
140+
132141
unit = zfcp_unit_find(port, zfcp_scsi_dev_lun(sdev));
133142
if (unit)
134143
put_device(&unit->dev);

drivers/s390/scsi/zfcp_sysfs.c

+48-6
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,53 @@ static ZFCP_DEV_ATTR(adapter, port_rescan, S_IWUSR, NULL,
235235

236236
DEFINE_MUTEX(zfcp_sysfs_port_units_mutex);
237237

238+
static void zfcp_sysfs_port_set_removing(struct zfcp_port *const port)
239+
{
240+
lockdep_assert_held(&zfcp_sysfs_port_units_mutex);
241+
atomic_set(&port->units, -1);
242+
}
243+
244+
bool zfcp_sysfs_port_is_removing(const struct zfcp_port *const port)
245+
{
246+
lockdep_assert_held(&zfcp_sysfs_port_units_mutex);
247+
return atomic_read(&port->units) == -1;
248+
}
249+
250+
static bool zfcp_sysfs_port_in_use(struct zfcp_port *const port)
251+
{
252+
struct zfcp_adapter *const adapter = port->adapter;
253+
unsigned long flags;
254+
struct scsi_device *sdev;
255+
bool in_use = true;
256+
257+
mutex_lock(&zfcp_sysfs_port_units_mutex);
258+
if (atomic_read(&port->units) > 0)
259+
goto unlock_port_units_mutex; /* zfcp_unit(s) under port */
260+
261+
spin_lock_irqsave(adapter->scsi_host->host_lock, flags);
262+
__shost_for_each_device(sdev, adapter->scsi_host) {
263+
const struct zfcp_scsi_dev *zsdev = sdev_to_zfcp(sdev);
264+
265+
if (sdev->sdev_state == SDEV_DEL ||
266+
sdev->sdev_state == SDEV_CANCEL)
267+
continue;
268+
if (zsdev->port != port)
269+
continue;
270+
/* alive scsi_device under port of interest */
271+
goto unlock_host_lock;
272+
}
273+
274+
/* port is about to be removed, so no more unit_add or slave_alloc */
275+
zfcp_sysfs_port_set_removing(port);
276+
in_use = false;
277+
278+
unlock_host_lock:
279+
spin_unlock_irqrestore(adapter->scsi_host->host_lock, flags);
280+
unlock_port_units_mutex:
281+
mutex_unlock(&zfcp_sysfs_port_units_mutex);
282+
return in_use;
283+
}
284+
238285
static ssize_t zfcp_sysfs_port_remove_store(struct device *dev,
239286
struct device_attribute *attr,
240287
const char *buf, size_t count)
@@ -257,16 +304,11 @@ static ssize_t zfcp_sysfs_port_remove_store(struct device *dev,
257304
else
258305
retval = 0;
259306

260-
mutex_lock(&zfcp_sysfs_port_units_mutex);
261-
if (atomic_read(&port->units) > 0) {
307+
if (zfcp_sysfs_port_in_use(port)) {
262308
retval = -EBUSY;
263-
mutex_unlock(&zfcp_sysfs_port_units_mutex);
264309
put_device(&port->dev); /* undo zfcp_get_port_by_wwpn() */
265310
goto out;
266311
}
267-
/* port is about to be removed, so no more unit_add */
268-
atomic_set(&port->units, -1);
269-
mutex_unlock(&zfcp_sysfs_port_units_mutex);
270312

271313
write_lock_irq(&adapter->port_list_lock);
272314
list_del(&port->list);

drivers/s390/scsi/zfcp_unit.c

+7-1
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun)
124124
int retval = 0;
125125

126126
mutex_lock(&zfcp_sysfs_port_units_mutex);
127-
if (atomic_read(&port->units) == -1) {
127+
if (zfcp_sysfs_port_is_removing(port)) {
128128
/* port is already gone */
129129
retval = -ENODEV;
130130
goto out;
@@ -168,8 +168,14 @@ int zfcp_unit_add(struct zfcp_port *port, u64 fcp_lun)
168168
write_lock_irq(&port->unit_list_lock);
169169
list_add_tail(&unit->list, &port->unit_list);
170170
write_unlock_irq(&port->unit_list_lock);
171+
/*
172+
* lock order: shost->scan_mutex before zfcp_sysfs_port_units_mutex
173+
* due to zfcp_unit_scsi_scan() => zfcp_scsi_slave_alloc()
174+
*/
175+
mutex_unlock(&zfcp_sysfs_port_units_mutex);
171176

172177
zfcp_unit_scsi_scan(unit);
178+
return retval;
173179

174180
out:
175181
mutex_unlock(&zfcp_sysfs_port_units_mutex);

0 commit comments

Comments
 (0)