From b32a88ad3782fbaa3cb851ebc1b694ba09840c86 Mon Sep 17 00:00:00 2001 From: Chris Leech Date: Thu, 19 Feb 2009 14:37:47 -0800 Subject: fcoe: fix handling of pending queue, prevent out of order frames References: bnc#477953 The per-lport pending queue handling can result in a frame being retried from a different CPU than it was originally sent from. The scope of the queue lock is not large enough to prevent reordering of frames within a sequence in that case. Before this change I would see out of order frames on large write IOs, leading to frequent exchange timeouts and retries. With the locking scope changed, the fcoe_insert_wait_queue(_head) helpers no longer do anything useful, so I removed them as well. Signed-off-by: Chris Leech Signed-off-by: Hannes Reinecke --- drivers/scsi/fcoe/libfcoe.c | 64 +++++++++---------------------------------- 1 files changed, 13 insertions(+), 51 deletions(-) diff --git a/drivers/scsi/fcoe/libfcoe.c b/drivers/scsi/fcoe/libfcoe.c index ada3ebb..d3f4eb9 100644 --- a/drivers/scsi/fcoe/libfcoe.c +++ b/drivers/scsi/fcoe/libfcoe.c @@ -71,8 +71,6 @@ struct fcoe_percpu_s *fcoe_percpu[NR_CPUS]; /* Function Prototyes */ static int fcoe_check_wait_queue(struct fc_lport *); -static void fcoe_insert_wait_queue_head(struct fc_lport *, struct sk_buff *); -static void fcoe_insert_wait_queue(struct fc_lport *, struct sk_buff *); static void fcoe_recv_flogi(struct fcoe_softc *, struct fc_frame *, u8 *); #ifdef CONFIG_HOTPLUG_CPU static int fcoe_cpu_callback(struct notifier_block *, ulong, void *); @@ -497,6 +495,7 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp) /* send down to lld */ fr_dev(fp) = lp; + spin_lock_bh(&fc->fcoe_pending_queue.lock); if (fc->fcoe_pending_queue.qlen) rc = fcoe_check_wait_queue(lp); @@ -504,10 +503,11 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp) rc = fcoe_start_io(skb); if (rc) { - fcoe_insert_wait_queue(lp, skb); + __skb_queue_tail(&fc->fcoe_pending_queue, skb); if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH) lp->qfull = 1; } + spin_unlock_bh(&fc->fcoe_pending_queue.lock); return 0; } @@ -732,8 +732,11 @@ void fcoe_watchdog(ulong vp) read_lock(&fcoe_hostlist_lock); list_for_each_entry(fc, &fcoe_hostlist, list) { - if (fc->lp) + if (fc->lp) { + spin_lock_bh(&fc->fcoe_pending_queue.lock); fcoe_check_wait_queue(fc->lp); + spin_unlock_bh(&fc->fcoe_pending_queue.lock); + } } read_unlock(&fcoe_hostlist_lock); @@ -756,6 +759,8 @@ void fcoe_watchdog(ulong vp) * in the wait_queue which will be emptied by the time function OR * by the next skb transmit. * + * Locking: fc->fcoe_pending_queue.lock must be held before calling + * * Returns: 0 for success **/ static int fcoe_check_wait_queue(struct fc_lport *lp) @@ -765,20 +770,17 @@ static int fcoe_check_wait_queue(struct fc_lport *lp) int rc = -1; fc = fcoe_softc(lp); - spin_lock_bh(&fc->fcoe_pending_queue.lock); if (fc->fcoe_pending_queue_active) goto out; fc->fcoe_pending_queue_active = 1; if (fc->fcoe_pending_queue.qlen) { while ((skb = __skb_dequeue(&fc->fcoe_pending_queue)) != NULL) { - spin_unlock_bh(&fc->fcoe_pending_queue.lock); rc = fcoe_start_io(skb); - if (rc) - fcoe_insert_wait_queue_head(lp, skb); - spin_lock_bh(&fc->fcoe_pending_queue.lock); - if (rc) + if (rc) { + __skb_queue_head(&fc->fcoe_pending_queue, skb); break; + } } /* * if interface pending queue is below FCOE_LOW_QUEUE_DEPTH @@ -790,47 +792,10 @@ static int fcoe_check_wait_queue(struct fc_lport *lp) fc->fcoe_pending_queue_active = 0; rc = fc->fcoe_pending_queue.qlen; out: - spin_unlock_bh(&fc->fcoe_pending_queue.lock); return rc; } /** - * fcoe_insert_wait_queue_head - puts skb to fcoe pending queue head - * @lp: the fc_port for this skb - * @skb: the associated skb to be xmitted - * - * Returns: none - **/ -static void fcoe_insert_wait_queue_head(struct fc_lport *lp, - struct sk_buff *skb) -{ - struct fcoe_softc *fc; - - fc = fcoe_softc(lp); - spin_lock_bh(&fc->fcoe_pending_queue.lock); - __skb_queue_head(&fc->fcoe_pending_queue, skb); - spin_unlock_bh(&fc->fcoe_pending_queue.lock); -} - -/** - * fcoe_insert_wait_queue - put the skb into fcoe pending queue tail - * @lp: the fc_port for this skb - * @skb: the associated skb to be xmitted - * - * Returns: none - **/ -static void fcoe_insert_wait_queue(struct fc_lport *lp, - struct sk_buff *skb) -{ - struct fcoe_softc *fc; - - fc = fcoe_softc(lp); - spin_lock_bh(&fc->fcoe_pending_queue.lock); - __skb_queue_tail(&fc->fcoe_pending_queue, skb); - spin_unlock_bh(&fc->fcoe_pending_queue.lock); -} - -/** * fcoe_dev_setup - setup link change notification interface * **/ @@ -1187,11 +1152,8 @@ void fcoe_clean_pending_queue(struct fc_lport *lp) struct sk_buff *skb; spin_lock_bh(&fc->fcoe_pending_queue.lock); - while ((skb = __skb_dequeue(&fc->fcoe_pending_queue)) != NULL) { - spin_unlock_bh(&fc->fcoe_pending_queue.lock); + while ((skb = __skb_dequeue(&fc->fcoe_pending_queue)) != NULL) kfree_skb(skb); - spin_lock_bh(&fc->fcoe_pending_queue.lock); - } spin_unlock_bh(&fc->fcoe_pending_queue.lock); } EXPORT_SYMBOL_GPL(fcoe_clean_pending_queue); -- 1.5.4.5