Skip to content

Commit 4da8c5f

Browse files
steffen-maiermartinkpetersen
authored andcommitted
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 18f87a6 ("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 828bc12 ("[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/[email protected] Fixes: 5ab944f ("[SCSI] zfcp: attach and release SAN nameserver port on demand") Cc: <[email protected]> #v2.6.28+ Reviewed-by: Benjamin Block <[email protected]> Signed-off-by: Steffen Maier <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
1 parent f323896 commit 4da8c5f

File tree

3 files changed

+26
-13
lines changed

3 files changed

+26
-13
lines changed

drivers/s390/scsi/zfcp_fc.c

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,27 +145,33 @@ void zfcp_fc_enqueue_event(struct zfcp_adapter *adapter,
145145

146146
static int zfcp_fc_wka_port_get(struct zfcp_fc_wka_port *wka_port)
147147
{
148+
int ret = -EIO;
149+
148150
if (mutex_lock_interruptible(&wka_port->mutex))
149151
return -ERESTARTSYS;
150152

151153
if (wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE ||
152154
wka_port->status == ZFCP_FC_WKA_PORT_CLOSING) {
153155
wka_port->status = ZFCP_FC_WKA_PORT_OPENING;
154-
if (zfcp_fsf_open_wka_port(wka_port))
156+
if (zfcp_fsf_open_wka_port(wka_port)) {
157+
/* could not even send request, nothing to wait for */
155158
wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE;
159+
goto out;
160+
}
156161
}
157162

158-
mutex_unlock(&wka_port->mutex);
159-
160-
wait_event(wka_port->completion_wq,
163+
wait_event(wka_port->opened,
161164
wka_port->status == ZFCP_FC_WKA_PORT_ONLINE ||
162165
wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE);
163166

164167
if (wka_port->status == ZFCP_FC_WKA_PORT_ONLINE) {
165168
atomic_inc(&wka_port->refcount);
166-
return 0;
169+
ret = 0;
170+
goto out;
167171
}
168-
return -EIO;
172+
out:
173+
mutex_unlock(&wka_port->mutex);
174+
return ret;
169175
}
170176

171177
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)
181187

182188
wka_port->status = ZFCP_FC_WKA_PORT_CLOSING;
183189
if (zfcp_fsf_close_wka_port(wka_port)) {
190+
/* could not even send request, nothing to wait for */
184191
wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE;
185-
wake_up(&wka_port->completion_wq);
192+
goto out;
186193
}
194+
wait_event(wka_port->closed,
195+
wka_port->status == ZFCP_FC_WKA_PORT_OFFLINE);
187196
out:
188197
mutex_unlock(&wka_port->mutex);
189198
}
@@ -193,13 +202,15 @@ static void zfcp_fc_wka_port_put(struct zfcp_fc_wka_port *wka_port)
193202
if (atomic_dec_return(&wka_port->refcount) != 0)
194203
return;
195204
/* wait 10 milliseconds, other reqs might pop in */
196-
schedule_delayed_work(&wka_port->work, HZ / 100);
205+
queue_delayed_work(wka_port->adapter->work_queue, &wka_port->work,
206+
msecs_to_jiffies(10));
197207
}
198208

199209
static void zfcp_fc_wka_port_init(struct zfcp_fc_wka_port *wka_port, u32 d_id,
200210
struct zfcp_adapter *adapter)
201211
{
202-
init_waitqueue_head(&wka_port->completion_wq);
212+
init_waitqueue_head(&wka_port->opened);
213+
init_waitqueue_head(&wka_port->closed);
203214

204215
wka_port->adapter = adapter;
205216
wka_port->d_id = d_id;

drivers/s390/scsi/zfcp_fc.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,8 @@ enum zfcp_fc_wka_status {
185185
/**
186186
* struct zfcp_fc_wka_port - representation of well-known-address (WKA) FC port
187187
* @adapter: Pointer to adapter structure this WKA port belongs to
188-
* @completion_wq: Wait for completion of open/close command
188+
* @opened: Wait for completion of open command
189+
* @closed: Wait for completion of close command
189190
* @status: Current status of WKA port
190191
* @refcount: Reference count to keep port open as long as it is in use
191192
* @d_id: FC destination id or well-known-address
@@ -195,7 +196,8 @@ enum zfcp_fc_wka_status {
195196
*/
196197
struct zfcp_fc_wka_port {
197198
struct zfcp_adapter *adapter;
198-
wait_queue_head_t completion_wq;
199+
wait_queue_head_t opened;
200+
wait_queue_head_t closed;
199201
enum zfcp_fc_wka_status status;
200202
atomic_t refcount;
201203
u32 d_id;

drivers/s390/scsi/zfcp_fsf.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1907,7 +1907,7 @@ static void zfcp_fsf_open_wka_port_handler(struct zfcp_fsf_req *req)
19071907
wka_port->status = ZFCP_FC_WKA_PORT_ONLINE;
19081908
}
19091909
out:
1910-
wake_up(&wka_port->completion_wq);
1910+
wake_up(&wka_port->opened);
19111911
}
19121912

19131913
/**
@@ -1966,7 +1966,7 @@ static void zfcp_fsf_close_wka_port_handler(struct zfcp_fsf_req *req)
19661966
}
19671967

19681968
wka_port->status = ZFCP_FC_WKA_PORT_OFFLINE;
1969-
wake_up(&wka_port->completion_wq);
1969+
wake_up(&wka_port->closed);
19701970
}
19711971

19721972
/**

0 commit comments

Comments
 (0)