Commit 55c8bafb authored by Chris Leech's avatar Chris Leech Committed by James Bottomley

[SCSI] fcoe: fix handling of pending queue, prevent out of order frames (v3)

In fcoe_check_wait_queue() the queue length could temporarily drop to 0,
before the last frame was successfully sent.  This resulted in out of order
data frames within a single sequence, leading to IO timeout errors.

This builds on the approach from Vasu Dev to only fix the queue management in
fcoe_check_wait_queue, where my first patch added locking to the transmit
path even when the pending queue was not in use.

This patch continues to use fcoe_pending_queue.qlen instead of introducing a
new length counter, but takes precautions to ensure it never drops to 0 before
the final frame in the queue has successfully been passed to the netdev qdisc
layer.  It also includes some cleanup of fcoe_check_wait_queue and removes the
fcoe_insert_wait_queue(_head) wrapper functions.
Signed-off-by: default avatarChris Leech <christopher.leech@intel.com>
Signed-off-by: default avatarRobert Love <robert.w.love@intel.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@HansenPartnership.com>
parent c826a314
...@@ -70,8 +70,6 @@ struct fcoe_percpu_s *fcoe_percpu[NR_CPUS]; ...@@ -70,8 +70,6 @@ struct fcoe_percpu_s *fcoe_percpu[NR_CPUS];
/* Function Prototyes */ /* Function Prototyes */
static int fcoe_check_wait_queue(struct fc_lport *); 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 *); static void fcoe_recv_flogi(struct fcoe_softc *, struct fc_frame *, u8 *);
#ifdef CONFIG_HOTPLUG_CPU #ifdef CONFIG_HOTPLUG_CPU
static int fcoe_cpu_callback(struct notifier_block *, ulong, void *); static int fcoe_cpu_callback(struct notifier_block *, ulong, void *);
...@@ -501,7 +499,9 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp) ...@@ -501,7 +499,9 @@ int fcoe_xmit(struct fc_lport *lp, struct fc_frame *fp)
rc = fcoe_start_io(skb); rc = fcoe_start_io(skb);
if (rc) { if (rc) {
fcoe_insert_wait_queue(lp, skb); spin_lock_bh(&fc->fcoe_pending_queue.lock);
__skb_queue_tail(&fc->fcoe_pending_queue, skb);
spin_unlock_bh(&fc->fcoe_pending_queue.lock);
if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH) if (fc->fcoe_pending_queue.qlen > FCOE_MAX_QUEUE_DEPTH)
lp->qfull = 1; lp->qfull = 1;
} }
...@@ -756,33 +756,36 @@ void fcoe_watchdog(ulong vp) ...@@ -756,33 +756,36 @@ void fcoe_watchdog(ulong vp)
*/ */
static int fcoe_check_wait_queue(struct fc_lport *lp) static int fcoe_check_wait_queue(struct fc_lport *lp)
{ {
struct fcoe_softc *fc = lport_priv(lp);
struct sk_buff *skb; struct sk_buff *skb;
struct fcoe_softc *fc;
int rc = -1; int rc = -1;
fc = lport_priv(lp);
spin_lock_bh(&fc->fcoe_pending_queue.lock); spin_lock_bh(&fc->fcoe_pending_queue.lock);
if (fc->fcoe_pending_queue_active) if (fc->fcoe_pending_queue_active)
goto out; goto out;
fc->fcoe_pending_queue_active = 1; fc->fcoe_pending_queue_active = 1;
if (fc->fcoe_pending_queue.qlen) {
while ((skb = __skb_dequeue(&fc->fcoe_pending_queue)) != NULL) { while (fc->fcoe_pending_queue.qlen) {
spin_unlock_bh(&fc->fcoe_pending_queue.lock); /* keep qlen > 0 until fcoe_start_io succeeds */
rc = fcoe_start_io(skb); fc->fcoe_pending_queue.qlen++;
if (rc) skb = __skb_dequeue(&fc->fcoe_pending_queue);
fcoe_insert_wait_queue_head(lp, skb);
spin_lock_bh(&fc->fcoe_pending_queue.lock); spin_unlock_bh(&fc->fcoe_pending_queue.lock);
if (rc) rc = fcoe_start_io(skb);
break; spin_lock_bh(&fc->fcoe_pending_queue.lock);
if (rc) {
__skb_queue_head(&fc->fcoe_pending_queue, skb);
/* undo temporary increment above */
fc->fcoe_pending_queue.qlen--;
break;
} }
/* /* undo temporary increment above */
* if interface pending queue is below FCOE_LOW_QUEUE_DEPTH fc->fcoe_pending_queue.qlen--;
* then clear qfull flag.
*/
if (fc->fcoe_pending_queue.qlen < FCOE_LOW_QUEUE_DEPTH)
lp->qfull = 0;
} }
if (fc->fcoe_pending_queue.qlen < FCOE_LOW_QUEUE_DEPTH)
lp->qfull = 0;
fc->fcoe_pending_queue_active = 0; fc->fcoe_pending_queue_active = 0;
rc = fc->fcoe_pending_queue.qlen; rc = fc->fcoe_pending_queue.qlen;
out: out:
...@@ -790,42 +793,6 @@ static int fcoe_check_wait_queue(struct fc_lport *lp) ...@@ -790,42 +793,6 @@ static int fcoe_check_wait_queue(struct fc_lport *lp)
return rc; 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 = lport_priv(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 = lport_priv(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 * fcoe_dev_setup() - setup link change notification interface
*/ */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment