1 From: Vasu Dev <vasu.dev@intel.com>
2 Subject: libfc, fcoe: fixed locking issues with lport->lp_mutex around lport->link_status
3 Patch-mainline: 6d235742e63f6b8912d8b200b75f9aa6d48f3e07
4 References: bnc #468053
6 The fcoe_xmit could call fc_pause in case the pending skb queue len is larger
7 than FCOE_MAX_QUEUE_DEPTH, the fc_pause was trying to grab lport->lp_muex to
8 change lport->link_status and that had these issues :-
10 1. The fcoe_xmit was getting called with bh disabled, thus causing
11 "BUG: scheduling while atomic" when grabbing lport->lp_muex with bh disabled.
13 2. fc_linkup and fc_linkdown function calls lport_enter function with
14 lport->lp_mutex held and these enter function in turn calls fcoe_xmit to send
15 lport related FC frame, e.g. fc_linkup => fc_lport_enter_flogi to send flogi
16 req. In this case grabbing the same lport->lp_mutex again in fc_puase from
17 fcoe_xmit would cause deadlock.
19 The lport->lp_mutex was used for setting FC_PAUSE in fcoe_xmit path but
20 FC_PAUSE bit was not used anywhere beside just setting and clear this
21 bit in lport->link_status, instead used a separate field qfull in fc_lport
22 to eliminate need for lport->lp_mutex to track pending queue full condition
23 and in turn avoid above described two locking issues.
25 Also added check for lp->qfull in fc_fcp_lport_queue_ready to trigger
26 SCSI_MLQUEUE_HOST_BUSY when lp->qfull is set to prevent more scsi-ml cmds
27 while lp->qfull is set.
29 This patch eliminated FC_LINK_UP and FC_PAUSE and instead used dedicated
30 fields in fc_lport for this, this simplified all related conditional
33 Also removed fc_pause and fc_unpause functions and instead used newly added
34 lport->qfull directly in fcoe.
36 Also fixed a circular locking in fc_exch_recv_abts.
38 These issues were blocking large file copy to a 2TB lun.
40 Signed-off-by: Vasu Dev <vasu.dev@intel.com>
41 Acked-by: Bernhard Walle <bwalle@suse.de>
43 drivers/scsi/fcoe/fcoe_sw.c | 6 +++---
44 drivers/scsi/fcoe/libfcoe.c | 41 +++++++++++++++++------------------------
45 drivers/scsi/libfc/fc_exch.c | 2 +-
46 drivers/scsi/libfc/fc_fcp.c | 6 +++---
47 drivers/scsi/libfc/fc_lport.c | 38 +++++++-------------------------------
48 drivers/scsi/libfc/fc_rport.c | 2 +-
49 include/scsi/libfc.h | 12 ++----------
50 7 files changed, 34 insertions(+), 73 deletions(-)
52 --- a/drivers/scsi/fcoe/fcoe_sw.c
53 +++ b/drivers/scsi/fcoe/fcoe_sw.c
54 @@ -116,7 +116,8 @@ static int fcoe_sw_lport_config(struct f
58 - lp->link_status = 0;
61 lp->max_retry_count = 3;
62 lp->e_d_tov = 2 * 1000; /* FC-FS default */
63 lp->r_a_tov = 2 * 2 * 1000;
64 @@ -181,9 +182,8 @@ static int fcoe_sw_netdev_config(struct
65 if (fc_set_mfs(lp, mfs))
68 - lp->link_status = ~FC_PAUSE & ~FC_LINK_UP;
69 if (!fcoe_link_ok(lp))
70 - lp->link_status |= FC_LINK_UP;
73 /* offload features support */
74 if (fc->real_dev->features & NETIF_F_SG)
75 --- a/drivers/scsi/fcoe/libfcoe.c
76 +++ b/drivers/scsi/fcoe/libfcoe.c
77 @@ -505,7 +505,7 @@ int fcoe_xmit(struct fc_lport *lp, struc
79 fcoe_insert_wait_queue(lp, skb);
80 if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
86 @@ -719,7 +719,7 @@ static void fcoe_recv_flogi(struct fcoe_
87 * fcoe_watchdog - fcoe timer callback
90 - * This checks the pending queue length for fcoe and put fcoe to be paused state
91 + * This checks the pending queue length for fcoe and set lport qfull
92 * if the FCOE_MAX_QUEUE_DEPTH is reached. This is done for all fc_lport on the
95 @@ -729,17 +729,17 @@ void fcoe_watchdog(ulong vp)
98 struct fcoe_softc *fc;
102 read_lock(&fcoe_hostlist_lock);
103 list_for_each_entry(fc, &fcoe_hostlist, list) {
106 if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
109 if (fcoe_check_wait_queue(lp) < FCOE_MAX_QUEUE_DEPTH) {
117 @@ -768,8 +768,7 @@ void fcoe_watchdog(ulong vp)
119 static int fcoe_check_wait_queue(struct fc_lport *lp)
121 - int rc, unpause = 0;
125 struct fcoe_softc *fc;
127 @@ -777,10 +776,10 @@ static int fcoe_check_wait_queue(struct
128 spin_lock_bh(&fc->fcoe_pending_queue.lock);
131 - * is this interface paused?
132 + * if interface pending queue full then set qfull in lport.
134 if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
137 if (fc->fcoe_pending_queue.qlen) {
138 while ((skb = __skb_dequeue(&fc->fcoe_pending_queue)) != NULL) {
139 spin_unlock_bh(&fc->fcoe_pending_queue.lock);
140 @@ -792,11 +791,9 @@ static int fcoe_check_wait_queue(struct
141 spin_lock_bh(&fc->fcoe_pending_queue.lock);
143 if (fc->fcoe_pending_queue.qlen < FCOE_MAX_QUEUE_DEPTH)
147 spin_unlock_bh(&fc->fcoe_pending_queue.lock);
148 - if ((unpause) && (paused))
150 return fc->fcoe_pending_queue.qlen;
153 @@ -874,7 +871,7 @@ static int fcoe_device_notification(stru
154 struct net_device *real_dev = ptr;
155 struct fcoe_softc *fc;
156 struct fcoe_dev_stats *stats;
162 @@ -891,17 +888,15 @@ static int fcoe_device_notification(stru
166 - new_status = lp->link_status;
167 + new_link_up = lp->link_up;
170 case NETDEV_GOING_DOWN:
171 - new_status &= ~FC_LINK_UP;
176 - new_status &= ~FC_LINK_UP;
177 - if (!fcoe_link_ok(lp))
178 - new_status |= FC_LINK_UP;
179 + new_link_up = !fcoe_link_ok(lp);
181 case NETDEV_CHANGEMTU:
182 mfs = fc->real_dev->mtu -
183 @@ -909,17 +904,15 @@ static int fcoe_device_notification(stru
184 sizeof(struct fcoe_crc_eof));
185 if (mfs >= FC_MIN_MAX_FRAME)
187 - new_status &= ~FC_LINK_UP;
188 - if (!fcoe_link_ok(lp))
189 - new_status |= FC_LINK_UP;
190 + new_link_up = !fcoe_link_ok(lp);
192 case NETDEV_REGISTER:
195 FC_DBG("unknown event %ld call", event);
197 - if (lp->link_status != new_status) {
198 - if ((new_status & FC_LINK_UP) == FC_LINK_UP)
199 + if (lp->link_up != new_link_up) {
203 stats = lp->dev_stats[smp_processor_id()];
204 --- a/drivers/scsi/libfc/fc_exch.c
205 +++ b/drivers/scsi/libfc/fc_exch.c
206 @@ -1096,7 +1096,7 @@ static void fc_exch_recv_abts(struct fc_
207 ap->ba_high_seq_cnt = fh->fh_seq_cnt;
208 ap->ba_low_seq_cnt = htons(sp->cnt);
210 - sp = fc_seq_start_next(sp);
211 + sp = fc_seq_start_next_locked(sp);
212 spin_unlock_bh(&ep->ex_lock);
213 fc_seq_send_last(sp, fp, FC_RCTL_BA_ACC, FC_TYPE_BLS);
214 fc_frame_free(rx_fp);
215 --- a/drivers/scsi/libfc/fc_fcp.c
216 +++ b/drivers/scsi/libfc/fc_fcp.c
220 #include <linux/module.h>
221 +#include <linux/delay.h>
222 #include <linux/kernel.h>
223 #include <linux/types.h>
224 #include <linux/spinlock.h>
225 #include <linux/scatterlist.h>
226 #include <linux/err.h>
227 #include <linux/crc32.h>
228 -#include <linux/delay.h>
230 #include <scsi/scsi_tcq.h>
231 #include <scsi/scsi.h>
232 @@ -1622,7 +1622,7 @@ out:
233 static inline int fc_fcp_lport_queue_ready(struct fc_lport *lp)
236 - return (lp->state == LPORT_ST_READY) && (lp->link_status & FC_LINK_UP);
237 + return (lp->state == LPORT_ST_READY) && lp->link_up && !lp->qfull;
241 @@ -1891,7 +1891,7 @@ int fc_eh_abort(struct scsi_cmnd *sc_cmd
242 lp = shost_priv(sc_cmd->device->host);
243 if (lp->state != LPORT_ST_READY)
245 - else if (!(lp->link_status & FC_LINK_UP))
246 + else if (!lp->link_up)
249 spin_lock_irqsave(lp->host->host_lock, flags);
250 --- a/drivers/scsi/libfc/fc_lport.c
251 +++ b/drivers/scsi/libfc/fc_lport.c
252 @@ -250,7 +250,7 @@ void fc_get_host_port_state(struct Scsi_
254 struct fc_lport *lp = shost_priv(shost);
256 - if ((lp->link_status & FC_LINK_UP) == FC_LINK_UP)
258 fc_host_port_state(shost) = FC_PORTSTATE_ONLINE;
260 fc_host_port_state(shost) = FC_PORTSTATE_OFFLINE;
261 @@ -484,7 +484,7 @@ static void fc_lport_recv_rnid_req(struc
262 * @sp: current sequence in the ADISC exchange
263 * @fp: ADISC request frame
265 - * Locking Note: The lport lock is exected to be held before calling
266 + * Locking Note: The lport lock is expected to be held before calling
269 static void fc_lport_recv_adisc_req(struct fc_seq *sp, struct fc_frame *in_fp,
270 @@ -577,8 +577,8 @@ void fc_linkup(struct fc_lport *lport)
271 fc_host_port_id(lport->host));
273 mutex_lock(&lport->lp_mutex);
274 - if ((lport->link_status & FC_LINK_UP) != FC_LINK_UP) {
275 - lport->link_status |= FC_LINK_UP;
276 + if (!lport->link_up) {
277 + lport->link_up = 1;
279 if (lport->state == LPORT_ST_RESET)
280 fc_lport_enter_flogi(lport);
281 @@ -597,8 +597,8 @@ void fc_linkdown(struct fc_lport *lport)
282 FC_DEBUG_LPORT("Link is down for port (%6x)\n",
283 fc_host_port_id(lport->host));
285 - if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP) {
286 - lport->link_status &= ~(FC_LINK_UP);
287 + if (lport->link_up) {
288 + lport->link_up = 0;
289 fc_lport_enter_reset(lport);
290 lport->tt.fcp_cleanup(lport);
292 @@ -607,30 +607,6 @@ void fc_linkdown(struct fc_lport *lport)
293 EXPORT_SYMBOL(fc_linkdown);
296 - * fc_pause - Pause the flow of frames
297 - * @lport: The lport to be paused
299 -void fc_pause(struct fc_lport *lport)
301 - mutex_lock(&lport->lp_mutex);
302 - lport->link_status |= FC_PAUSE;
303 - mutex_unlock(&lport->lp_mutex);
305 -EXPORT_SYMBOL(fc_pause);
308 - * fc_unpause - Unpause the flow of frames
309 - * @lport: The lport to be unpaused
311 -void fc_unpause(struct fc_lport *lport)
313 - mutex_lock(&lport->lp_mutex);
314 - lport->link_status &= ~(FC_PAUSE);
315 - mutex_unlock(&lport->lp_mutex);
317 -EXPORT_SYMBOL(fc_unpause);
320 * fc_fabric_logoff - Logout of the fabric
321 * @lport: fc_lport pointer to logoff the fabric
323 @@ -977,7 +953,7 @@ static void fc_lport_enter_reset(struct
324 fc_host_fabric_name(lport->host) = 0;
325 fc_host_port_id(lport->host) = 0;
327 - if ((lport->link_status & FC_LINK_UP) == FC_LINK_UP)
328 + if (lport->link_up)
329 fc_lport_enter_flogi(lport);
332 --- a/drivers/scsi/libfc/fc_rport.c
333 +++ b/drivers/scsi/libfc/fc_rport.c
334 @@ -425,7 +425,7 @@ static void fc_rport_error(struct fc_rpo
335 PTR_ERR(fp), fc_rport_state(rport), rdata->retries);
337 if (!fp || PTR_ERR(fp) == -FC_EX_TIMEOUT) {
340 * Memory allocation failure, or the exchange timed out.
343 --- a/include/scsi/libfc.h
344 +++ b/include/scsi/libfc.h
349 -#define FC_PAUSE (1 << 1)
350 -#define FC_LINK_UP (1 << 0)
352 enum fc_lport_state {
355 @@ -603,7 +600,8 @@ struct fc_lport {
357 /* Operational Information */
358 struct libfc_function_template tt;
362 enum fc_lport_state state;
363 unsigned long boot_time;
365 @@ -704,12 +702,6 @@ void fc_linkup(struct fc_lport *);
366 void fc_linkdown(struct fc_lport *);
369 - * Pause and unpause traffic.
371 -void fc_pause(struct fc_lport *);
372 -void fc_unpause(struct fc_lport *);
375 * Configure the local port.
377 int fc_lport_config(struct fc_lport *);