[PATCH 00/19] libfc, libfcoe, and fcoe updates for 3.13

[PATCH 00/19] libfc, libfcoe, and fcoe updates for 3.13

[Robert Love]

The following series implements some fixes and enhancements.

All but the last 3 patches are a resend from
patches sent at the last (3.12) merge window.

There is no particular theme to this series, just some
code optimizations and bug fixes.

A 'request-pull' e-mail will follow this series.
Please pull when appropriate.

---

Bart Van Assche (13):
      libfc: Source code comment spelling fixes
      libfc: Debug code fixes
      libfc: Micro-optimize fc_setup_exch_mgr()
      libfc: Clarify fc_exch_find()
      libfc: Fix a race in fc_exch_timer_set_locked()
      libfc: Protect ep->esb_stat changes via ex_lock
      libfc: Avoid that sending after an abort triggers a kernel warning
      libfc: Reduce exchange lock contention in fc_exch_recv_abts()
      libfc: Do not invoke the response handler after fc_exch_done()
      fcp: Do not interpret check condition as underrun
      fcoe: Declare fcoe_ctlr_mode_set() static
      fcoe: Add missing newlines in debug messages
      fcoe: Reduce fcoe_sysfs_fcf_add() stack usage

Joe Perches (1):
      scsi: Convert uses of compare_ether_addr to ether_addr_equal

Neil Horman (4):
      fcoe: ensure that skb placed on the fip_recv_list are unshared
      fcoe: make sure fcoe frames are unshared prior to manipulating them
      fcoe: cleanup return codes from fcoe_rcv
      fcoe: Fix missing mutex_unlock in fcoe_sysfs_fcf_add error path

Robert Love (1):
      libfcoe: Make fcoe_sysfs optional / fix fnic NULL exception


 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    4 -
 drivers/scsi/fcoe/fcoe.c          |   25 ++--
 drivers/scsi/fcoe/fcoe_ctlr.c     |  148 ++++++++++++++--------
 drivers/scsi/fcoe/fcoe_sysfs.c    |   14 +-
 drivers/scsi/fnic/fnic_fcs.c      |    6 -
 drivers/scsi/libfc/fc_exch.c      |  251 ++++++++++++++++++++++++-------------
 drivers/scsi/libfc/fc_fcp.c       |   10 +
 drivers/scsi/libfc/fc_lport.c     |    4 -
 drivers/scsi/libfc/fc_rport.c     |    6 -
 include/scsi/fc/fc_fc2.h          |    2 
 include/scsi/libfc.h              |    9 +
 include/scsi/libfcoe.h            |    7 +
 12 files changed, 310 insertions(+), 176 deletions(-)

-- 
Thanks, //Rob

[PATCH 01/19] fcoe: ensure that skb placed on the fip_recv_list are unshared

[Robert Love]

From: Neil Horman <nhorman@tuxdriver.com>

Recently had this Oops reported to me on the 3.10 kernel:

[  807.554955] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
[  807.562799] IP: [<ffffffff814e6fc7>] skb_dequeue+0x47/0x70
[  807.568296] PGD 20c889067 PUD 20c8b8067 PMD 0
[  807.572769] Oops: 0002 [#1] SMP
[  807.655597] Hardware name: Dell Inc. PowerEdge R415/0DDT2D, BIOS 1.8.6 12/06/2011
[  807.663079] Workqueue: events fcoe_ctlr_recv_work [libfcoe]
[  807.668656] task: ffff88020b42a160 ti: ffff88020ae6c000 task.ti: ffff88020ae6c000
[  807.676126] RIP: 0010:[<ffffffff814e6fc7>]  [<ffffffff814e6fc7>] skb_dequeue+0x47/0x70
[  807.684046] RSP: 0000:ffff88020ae6dd70  EFLAGS: 00010097
[  807.689349] RAX: 0000000000000246 RBX: ffff8801d04d6700 RCX: 0000000000000000
[  807.696474] RDX: 0000000000000000 RSI: 0000000000000246 RDI: ffff88020df26434
[  807.703598] RBP: ffff88020ae6dd88 R08: 00000000000173e0 R09: ffff880216e173e0
[  807.710723] R10: ffffffff814e5897 R11: ffffea0007413580 R12: ffff88020df26420
[  807.717847] R13: ffff88020df26434 R14: 0000000000000004 R15: ffff8801d04c42ce
[  807.724972] FS:  00007fdaab6048c0(0000) GS:ffff880216e00000(0000) knlGS:0000000000000000
[  807.733049] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  807.738785] CR2: 0000000000000008 CR3: 000000020cbc9000 CR4: 00000000000006f0
[  807.745910] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[  807.753033] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[  807.760156] Stack:
[  807.762162]  ffff8801d04d6700 0000000000000001 ffff88020df26400 ffff88020ae6de20
[  807.769586]  ffffffffa0444409 ffff88020b046a00 ffff88020ae6dde8 ffffffff810105be
[  807.777008]  ffff88020b42a868 0000000000000000 ffff88020df264a8 ffff88020df26348
[  807.784431] Call Trace:
[  807.786885]  [<ffffffffa0444409>] fcoe_ctlr_recv_work+0x59/0x9a0 [libfcoe]
[  807.793755]  [<ffffffff810105be>] ? __switch_to+0x13e/0x4a0
[  807.799324]  [<ffffffff8107d0e6>] process_one_work+0x176/0x420
[  807.805151]  [<ffffffff8107dd0b>] worker_thread+0x11b/0x3a0
[  807.810717]  [<ffffffff8107dbf0>] ? rescuer_thread+0x350/0x350
[  807.816545]  [<ffffffff810842b0>] kthread+0xc0/0xd0
[  807.821416]  [<ffffffff810841f0>] ? insert_kthread_work+0x40/0x40
[  807.827503]  [<ffffffff8160ce2c>] ret_from_fork+0x7c/0xb0
[  807.832897]  [<ffffffff810841f0>] ? insert_kthread_work+0x40/0x40
[  807.858500] RIP  [<ffffffff814e6fc7>] skb_dequeue+0x47/0x70
[  807.864076]  RSP <ffff88020ae6dd70>
[  807.867558] CR2: 0000000000000008

Looks like the root cause is the fact that the packet recieve function
fcoe_ctlr_recv enqueues the skb to a sk_buff_head_list prior to ensuring that
the skb is unshared.  This can happen when multiple packet listeners recieve an
skb, as the deliver_skb function just increments skb->users for each handler.
As a result, having multiple users of a single skb results in multiple
manipulators of its methods, implying list corruption, and the oops recorded
above.

The fix is pretty easy, just make sure that we clone the skb if its got multiple
users with the skb_share_check function, like other protocols do.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c |    3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 203415e..35b1fb7 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -1453,6 +1453,9 @@ err:
  */
 void fcoe_ctlr_recv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 {
+	skb = skb_share_check(skb, GFP_ATOMIC);
+	if (!skb)
+		return;
 	skb_queue_tail(&fip->fip_recv_list, skb);
 	schedule_work(&fip->recv_work);
 }

[PATCH 02/19] fcoe: make sure fcoe frames are unshared prior to manipulating them

[Robert Love]

From: Neil Horman <nhorman@tuxdriver.com>

Based on my last patch I noticed that fcoe_rcv has a simmilar problem, in that
it manipulates the passed in skb without checking to see if it has other users.
Making manipulations to a shared skb can result in various corruptions.

Easy fix, just make sure the skb is unshared prior to doing anything with it.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 07453bb..f9b0302 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1452,6 +1452,12 @@ static int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 			skb_tail_pointer(skb), skb_end_pointer(skb),
 			skb->csum, skb->dev ? skb->dev->name : "<NULL>");
 
+
+	skb = skb_share_check(skb, GFP_ATOMIC);
+
+	if (skb == NULL)
+		return NET_RX_DROP;
+
 	eh = eth_hdr(skb);
 
 	if (is_fip_mode(ctlr) &&

[PATCH 03/19] fcoe: cleanup return codes from fcoe_rcv

[Robert Love]

From: Neil Horman <nhorman@tuxdriver.com>

the return codes from fcoe_rcv should be NET_RX_*, not 0 or -1.

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index f9b0302..134ca3b 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1546,13 +1546,13 @@ static int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 		wake_up_process(fps->thread);
 	spin_unlock(&fps->fcoe_rx_list.lock);
 
-	return 0;
+	return NET_RX_SUCCESS;
 err:
 	per_cpu_ptr(lport->stats, get_cpu())->ErrorFrames++;
 	put_cpu();
 err2:
 	kfree_skb(skb);
-	return -1;
+	return NET_RX_DROP;
 }
 
 /**

[PATCH 04/19] libfc: Source code comment spelling fixes

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

Change 'initiaive' into 'initiative', 'remainig' into 'remaining'
and change 'exected' into 'expected'.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c  |    5 +----
 drivers/scsi/libfc/fc_lport.c |    2 +-
 drivers/scsi/libfc/fc_rport.c |    6 +++---
 include/scsi/fc/fc_fc2.h      |    2 +-
 4 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 5879929..b233a2d 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -303,10 +303,7 @@ static void fc_exch_setup_hdr(struct fc_exch *ep, struct fc_frame *fp,
 		fr_eof(fp) = FC_EOF_N;
 	}
 
-	/*
-	 * Initialize remainig fh fields
-	 * from fc_fill_fc_hdr
-	 */
+	/* Initialize remaining fh fields from fc_fill_fc_hdr */
 	fh->fh_ox_id = htons(ep->oxid);
 	fh->fh_rx_id = htons(ep->rxid);
 	fh->fh_seq_id = ep->seq.id;
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index f04d15c..a32f314 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -516,7 +516,7 @@ static void fc_lport_recv_rnid_req(struct fc_lport *lport,
  * @lport: The local port receiving the LOGO
  * @fp:	   The LOGO request frame
  *
- * Locking Note: The lport lock is exected to be held before calling
+ * Locking Note: The lport lock is expected to be held before calling
  * this function.
  */
 static void fc_lport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp)
diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
index c710d90..589ff9a 100644
--- a/drivers/scsi/libfc/fc_rport.c
+++ b/drivers/scsi/libfc/fc_rport.c
@@ -1705,7 +1705,7 @@ reject:
  * @rdata: The remote port that sent the PRLI request
  * @rx_fp: The PRLI request frame
  *
- * Locking Note: The rport lock is exected to be held before calling
+ * Locking Note: The rport lock is expected to be held before calling
  * this function.
  */
 static void fc_rport_recv_prli_req(struct fc_rport_priv *rdata,
@@ -1824,7 +1824,7 @@ drop:
  * @rdata: The remote port that sent the PRLO request
  * @rx_fp: The PRLO request frame
  *
- * Locking Note: The rport lock is exected to be held before calling
+ * Locking Note: The rport lock is expected to be held before calling
  * this function.
  */
 static void fc_rport_recv_prlo_req(struct fc_rport_priv *rdata,
@@ -1895,7 +1895,7 @@ drop:
  * @lport: The local port that received the LOGO request
  * @fp:	   The LOGO request frame
  *
- * Locking Note: The rport lock is exected to be held before calling
+ * Locking Note: The rport lock is expected to be held before calling
  * this function.
  */
 static void fc_rport_recv_logo_req(struct fc_lport *lport, struct fc_frame *fp)
diff --git a/include/scsi/fc/fc_fc2.h b/include/scsi/fc/fc_fc2.h
index f87777d..0b26714 100644
--- a/include/scsi/fc/fc_fc2.h
+++ b/include/scsi/fc/fc_fc2.h
@@ -104,7 +104,7 @@ struct fc_esb {
  * esb_e_stat - flags from FC-FS-2 T11/1619-D Rev 0.90.
  */
 #define ESB_ST_RESP         (1 << 31)   /* responder to exchange */
-#define ESB_ST_SEQ_INIT     (1 << 30)   /* port holds sequence initiaive */
+#define ESB_ST_SEQ_INIT     (1 << 30)   /* port holds sequence initiative */
 #define ESB_ST_COMPLETE     (1 << 29)   /* exchange is complete */
 #define ESB_ST_ABNORMAL     (1 << 28)   /* abnormal ending condition */
 #define ESB_ST_REC_QUAL     (1 << 26)   /* recovery qualifier active */

[PATCH 05/19] libfc: Debug code fixes

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

The second argument of fc_lport_error() may be a valid frame pointer.
Hence only print it as an error code if it really is an error code.

Debug statements must end in a newline. Add one where it is missing.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c  |    8 ++++----
 drivers/scsi/libfc/fc_lport.c |    2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index b233a2d..cb2b900 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1659,7 +1659,7 @@ static void fc_exch_recv_bls(struct fc_exch_mgr *mp, struct fc_frame *fp)
 			break;
 		default:
 			if (ep)
-				FC_EXCH_DBG(ep, "BLS rctl %x - %s received",
+				FC_EXCH_DBG(ep, "BLS rctl %x - %s received\n",
 					    fh->fh_r_ctl,
 					    fc_exch_rctl_name(fh->fh_r_ctl));
 			break;
@@ -1953,13 +1953,13 @@ static void fc_exch_rrq_resp(struct fc_seq *sp, struct fc_frame *fp, void *arg)
 
 	switch (op) {
 	case ELS_LS_RJT:
-		FC_EXCH_DBG(aborted_ep, "LS_RJT for RRQ");
+		FC_EXCH_DBG(aborted_ep, "LS_RJT for RRQ\n");
 		/* fall through */
 	case ELS_LS_ACC:
 		goto cleanup;
 	default:
-		FC_EXCH_DBG(aborted_ep, "unexpected response op %x "
-			    "for RRQ", op);
+		FC_EXCH_DBG(aborted_ep, "unexpected response op %x for RRQ\n",
+			    op);
 		return;
 	}
 
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c
index a32f314..e01a298 100644
--- a/drivers/scsi/libfc/fc_lport.c
+++ b/drivers/scsi/libfc/fc_lport.c
@@ -1088,7 +1088,7 @@ static void fc_lport_error(struct fc_lport *lport, struct fc_frame *fp)
 {
 	unsigned long delay = 0;
 	FC_LPORT_DBG(lport, "Error %ld in state %s, retries %d\n",
-		     PTR_ERR(fp), fc_lport_state(lport),
+		     IS_ERR(fp) ? -PTR_ERR(fp) : 0, fc_lport_state(lport),
 		     lport->retry_count);
 
 	if (PTR_ERR(fp) == -FC_EX_CLOSED)

[PATCH 06/19] libfc: Micro-optimize fc_setup_exch_mgr()

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

Convert a loop into an ilog2() call. Although this code is not performance
sensitive this conversion makes this code easier to read.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |   10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index cb2b900..d0be52a 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -27,6 +27,7 @@
 #include <linux/slab.h>
 #include <linux/err.h>
 #include <linux/export.h>
+#include <linux/log2.h>
 
 #include <scsi/fc/fc_fc2.h>
 
@@ -2530,13 +2531,8 @@ int fc_setup_exch_mgr(void)
 	 * cpu on which exchange originated by simple bitwise
 	 * AND operation between fc_cpu_mask and exchange id.
 	 */
-	fc_cpu_mask = 1;
-	fc_cpu_order = 0;
-	while (fc_cpu_mask < nr_cpu_ids) {
-		fc_cpu_mask <<= 1;
-		fc_cpu_order++;
-	}
-	fc_cpu_mask--;
+	fc_cpu_order = ilog2(roundup_pow_of_two(nr_cpu_ids));
+	fc_cpu_mask = (1 << fc_cpu_order) - 1;
 
 	fc_exch_workqueue = create_singlethread_workqueue("fc_exch_workqueue");
 	if (!fc_exch_workqueue)

[PATCH 07/19] libfc: Clarify fc_exch_find()

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

The condition ep != NULL && ep->xid != xid can never be met. Make
this explicit.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index d0be52a..f6bb0fb 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -836,8 +836,10 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr *mp, u16 xid)
 		pool = per_cpu_ptr(mp->pool, xid & fc_cpu_mask);
 		spin_lock_bh(&pool->lock);
 		ep = fc_exch_ptr_get(pool, (xid - mp->min_xid) >> fc_cpu_order);
-		if (ep && ep->xid == xid)
+		if (ep) {
+			WARN_ON(ep->xid != xid);
 			fc_exch_hold(ep);
+		}
 		spin_unlock_bh(&pool->lock);
 	}
 	return ep;

[PATCH 08/19] libfc: Fix a race in fc_exch_timer_set_locked()

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

It is allowed to pass a zero timeout value to fc_seq_exch_abort().
Avoid that this can cause the timeout function to drop the exchange
reference before it has been increased by fc_exch_timer_set_locked().
This patch fixes a crash when running FCoE target code with poisoning
enabled in the memory allocator.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |    7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index f6bb0fb..7000203 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -360,9 +360,10 @@ static inline void fc_exch_timer_set_locked(struct fc_exch *ep,
 
 	FC_EXCH_DBG(ep, "Exchange timer armed : %d msecs\n", timer_msec);
 
-	if (queue_delayed_work(fc_exch_workqueue, &ep->timeout_work,
-			       msecs_to_jiffies(timer_msec)))
-		fc_exch_hold(ep);		/* hold for timer */
+	fc_exch_hold(ep);		/* hold for timer */
+	if (!queue_delayed_work(fc_exch_workqueue, &ep->timeout_work,
+				msecs_to_jiffies(timer_msec)))
+		fc_exch_release(ep);
 }
 
 /**

[PATCH 09/19] libfc: Protect ep->esb_stat changes via ex_lock

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

This patch avoids that the WARN_ON(!(ep->esb_stat & ESB_ST_SEQ_INIT))
statement in fc_seq_send_locked() gets triggered sporadically when
running FCoE target code due to concurrent ep->esb_stat modifications.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 7000203..bc0aba4 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -988,6 +988,7 @@ static enum fc_pf_rjt_reason fc_seq_lookup_recip(struct fc_lport *lport,
 		}
 	}
 
+	spin_lock_bh(&ep->ex_lock);
 	/*
 	 * At this point, we have the exchange held.
 	 * Find or create the sequence.
@@ -1015,11 +1016,11 @@ static enum fc_pf_rjt_reason fc_seq_lookup_recip(struct fc_lport *lport,
 				 * sending RSP, hence write request on other
 				 * end never finishes.
 				 */
-				spin_lock_bh(&ep->ex_lock);
 				sp->ssb_stat |= SSB_ST_RESP;
 				sp->id = fh->fh_seq_id;
-				spin_unlock_bh(&ep->ex_lock);
 			} else {
+				spin_unlock_bh(&ep->ex_lock);
+
 				/* sequence/exch should exist */
 				reject = FC_RJT_SEQ_ID;
 				goto rel;
@@ -1030,6 +1031,7 @@ static enum fc_pf_rjt_reason fc_seq_lookup_recip(struct fc_lport *lport,
 
 	if (f_ctl & FC_FC_SEQ_INIT)
 		ep->esb_stat |= ESB_ST_SEQ_INIT;
+	spin_unlock_bh(&ep->ex_lock);
 
 	fr_seq(fp) = sp;
 out:
@@ -1479,8 +1481,11 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 
 	f_ctl = ntoh24(fh->fh_f_ctl);
 	fr_seq(fp) = sp;
+
+	spin_lock_bh(&ep->ex_lock);
 	if (f_ctl & FC_FC_SEQ_INIT)
 		ep->esb_stat |= ESB_ST_SEQ_INIT;
+	spin_unlock_bh(&ep->ex_lock);
 
 	if (fc_sof_needs_ack(sof))
 		fc_seq_send_ack(sp, fp);

[PATCH 10/19] libfc: Avoid that sending after an abort triggers a kernel warning

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

Calling fc_seq_send() after an ABTS message has been received triggers
a kernel warning (WARN_ON(!(ep->esb_stat & ESB_ST_SEQ_INIT))). Avoid
this by returning -ENXIO to the caller if fc_seq_send() is invoked after
an ABTS message has been received.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |   59 ++++++++++++++++++++++++++----------------
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index bc0aba4..2a7fd5a 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -463,15 +463,21 @@ static void fc_exch_delete(struct fc_exch *ep)
 }
 
 static int fc_seq_send_locked(struct fc_lport *lport, struct fc_seq *sp,
-		       struct fc_frame *fp)
+			      struct fc_frame *fp)
 {
 	struct fc_exch *ep;
 	struct fc_frame_header *fh = fc_frame_header_get(fp);
-	int error;
+	int error = -ENXIO;
 	u32 f_ctl;
 	u8 fh_type = fh->fh_type;
 
 	ep = fc_seq_exch(sp);
+
+	if (ep->esb_stat & (ESB_ST_COMPLETE | ESB_ST_ABNORMAL)) {
+		fc_frame_free(fp);
+		goto out;
+	}
+
 	WARN_ON(!(ep->esb_stat & ESB_ST_SEQ_INIT));
 
 	f_ctl = ntoh24(fh->fh_f_ctl);
@@ -514,6 +520,9 @@ out:
  * @lport: The local port that the exchange will be sent on
  * @sp:	   The sequence to be sent
  * @fp:	   The frame to be sent on the exchange
+ *
+ * Note: The frame will be freed either by a direct call to fc_frame_free(fp)
+ * or indirectly by calling libfc_function_template.frame_send().
  */
 static int fc_seq_send(struct fc_lport *lport, struct fc_seq *sp,
 		       struct fc_frame *fp)
@@ -621,27 +630,31 @@ static int fc_exch_abort_locked(struct fc_exch *ep,
 	if (!sp)
 		return -ENOMEM;
 
-	ep->esb_stat |= ESB_ST_SEQ_INIT | ESB_ST_ABNORMAL;
 	if (timer_msec)
 		fc_exch_timer_set_locked(ep, timer_msec);
 
-	/*
-	 * If not logged into the fabric, don't send ABTS but leave
-	 * sequence active until next timeout.
-	 */
-	if (!ep->sid)
-		return 0;
-
-	/*
-	 * Send an abort for the sequence that timed out.
-	 */
-	fp = fc_frame_alloc(ep->lp, 0);
-	if (fp) {
-		fc_fill_fc_hdr(fp, FC_RCTL_BA_ABTS, ep->did, ep->sid,
-			       FC_TYPE_BLS, FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0);
-		error = fc_seq_send_locked(ep->lp, sp, fp);
-	} else
-		error = -ENOBUFS;
+	if (ep->sid) {
+		/*
+		 * Send an abort for the sequence that timed out.
+		 */
+		fp = fc_frame_alloc(ep->lp, 0);
+		if (fp) {
+			ep->esb_stat |= ESB_ST_SEQ_INIT;
+			fc_fill_fc_hdr(fp, FC_RCTL_BA_ABTS, ep->did, ep->sid,
+				       FC_TYPE_BLS, FC_FC_END_SEQ |
+				       FC_FC_SEQ_INIT, 0);
+			error = fc_seq_send_locked(ep->lp, sp, fp);
+		} else {
+			error = -ENOBUFS;
+		}
+	} else {
+		/*
+		 * If not logged into the fabric, don't send ABTS but leave
+		 * sequence active until next timeout.
+		 */
+		error = 0;
+	}
+	ep->esb_stat |= ESB_ST_ABNORMAL;
 	return error;
 }
 
@@ -1299,9 +1312,10 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 		spin_unlock_bh(&ep->ex_lock);
 		goto reject;
 	}
-	if (!(ep->esb_stat & ESB_ST_REC_QUAL))
+	if (!(ep->esb_stat & ESB_ST_REC_QUAL)) {
+		ep->esb_stat |= ESB_ST_REC_QUAL;
 		fc_exch_hold(ep);		/* hold for REC_QUAL */
-	ep->esb_stat |= ESB_ST_ABNORMAL | ESB_ST_REC_QUAL;
+	}
 	fc_exch_timer_set_locked(ep, ep->r_a_tov);
 
 	fp = fc_frame_alloc(ep->lp, sizeof(*ap));
@@ -1322,6 +1336,7 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 	}
 	sp = fc_seq_start_next_locked(sp);
 	fc_seq_send_last(sp, fp, FC_RCTL_BA_ACC, FC_TYPE_BLS);
+	ep->esb_stat |= ESB_ST_ABNORMAL;
 	spin_unlock_bh(&ep->ex_lock);
 	fc_frame_free(rx_fp);
 	return;

[PATCH 11/19] libfc: Reduce exchange lock contention in fc_exch_recv_abts()

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

Reduce the time during which the exchange lock is held by allocating
a frame before obtaining the exchange lock.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |   18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 2a7fd5a..47ebc7b 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1307,9 +1307,16 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 
 	if (!ep)
 		goto reject;
+
+	fp = fc_frame_alloc(ep->lp, sizeof(*ap));
+	if (!fp)
+		goto free;
+
 	spin_lock_bh(&ep->ex_lock);
 	if (ep->esb_stat & ESB_ST_COMPLETE) {
 		spin_unlock_bh(&ep->ex_lock);
+
+		fc_frame_free(fp);
 		goto reject;
 	}
 	if (!(ep->esb_stat & ESB_ST_REC_QUAL)) {
@@ -1317,12 +1324,6 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 		fc_exch_hold(ep);		/* hold for REC_QUAL */
 	}
 	fc_exch_timer_set_locked(ep, ep->r_a_tov);
-
-	fp = fc_frame_alloc(ep->lp, sizeof(*ap));
-	if (!fp) {
-		spin_unlock_bh(&ep->ex_lock);
-		goto free;
-	}
 	fh = fc_frame_header_get(fp);
 	ap = fc_frame_payload_get(fp, sizeof(*ap));
 	memset(ap, 0, sizeof(*ap));
@@ -1338,13 +1339,14 @@ static void fc_exch_recv_abts(struct fc_exch *ep, struct fc_frame *rx_fp)
 	fc_seq_send_last(sp, fp, FC_RCTL_BA_ACC, FC_TYPE_BLS);
 	ep->esb_stat |= ESB_ST_ABNORMAL;
 	spin_unlock_bh(&ep->ex_lock);
+
+free:
 	fc_frame_free(rx_fp);
 	return;
 
 reject:
 	fc_exch_send_ba_rjt(rx_fp, FC_BA_RJT_UNABLE, FC_BA_RJT_INV_XID);
-free:
-	fc_frame_free(rx_fp);
+	goto free;
 }
 
 /**

[PATCH 12/19] libfc: Do not invoke the response handler after fc_exch_done()

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

While the FCoE initiator driver invokes fc_exch_done() from inside
the libfc response handler, FCoE target drivers typically invoke
fc_exch_done() from outside the libfc response handler. The object
fc_exch.arg points at may disappear as soon as fc_exch_done() has
finished. So it's important not to invoke the response handler
function after fc_exch_done() has finished. Modify libfc such that
this guarantee is provided if fc_exch_done() is invoked from
outside a response handler. This patch fixes a sporadic crash in
FCoE target implementations after a command has been aborted.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_exch.c |  131 +++++++++++++++++++++++++++++-------------
 include/scsi/libfc.h         |    9 +++
 2 files changed, 101 insertions(+), 39 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 47ebc7b..1b3a094 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -381,6 +381,8 @@ static void fc_exch_timer_set(struct fc_exch *ep, unsigned int timer_msec)
 /**
  * fc_exch_done_locked() - Complete an exchange with the exchange lock held
  * @ep: The exchange that is complete
+ *
+ * Note: May sleep if invoked from outside a response handler.
  */
 static int fc_exch_done_locked(struct fc_exch *ep)
 {
@@ -392,7 +394,6 @@ static int fc_exch_done_locked(struct fc_exch *ep)
 	 * ep, and in that case we only clear the resp and set it as
 	 * complete, so it can be reused by the timer to send the rrq.
 	 */
-	ep->resp = NULL;
 	if (ep->state & FC_EX_DONE)
 		return rc;
 	ep->esb_stat |= ESB_ST_COMPLETE;
@@ -589,6 +590,8 @@ static struct fc_seq *fc_seq_start_next(struct fc_seq *sp)
 
 /*
  * Set the response handler for the exchange associated with a sequence.
+ *
+ * Note: May sleep if invoked from outside a response handler.
  */
 static void fc_seq_set_resp(struct fc_seq *sp,
 			    void (*resp)(struct fc_seq *, struct fc_frame *,
@@ -596,8 +599,18 @@ static void fc_seq_set_resp(struct fc_seq *sp,
 			    void *arg)
 {
 	struct fc_exch *ep = fc_seq_exch(sp);
+	DEFINE_WAIT(wait);
 
 	spin_lock_bh(&ep->ex_lock);
+	while (ep->resp_active && ep->resp_task != current) {
+		prepare_to_wait(&ep->resp_wq, &wait, TASK_UNINTERRUPTIBLE);
+		spin_unlock_bh(&ep->ex_lock);
+
+		schedule();
+
+		spin_lock_bh(&ep->ex_lock);
+	}
+	finish_wait(&ep->resp_wq, &wait);
 	ep->resp = resp;
 	ep->arg = arg;
 	spin_unlock_bh(&ep->ex_lock);
@@ -681,6 +694,61 @@ static int fc_seq_exch_abort(const struct fc_seq *req_sp,
 }
 
 /**
+ * fc_invoke_resp() - invoke ep->resp()
+ *
+ * Notes:
+ * It is assumed that after initialization finished (this means the
+ * first unlock of ex_lock after fc_exch_alloc()) ep->resp and ep->arg are
+ * modified only via fc_seq_set_resp(). This guarantees that none of these
+ * two variables changes if ep->resp_active > 0.
+ *
+ * If an fc_seq_set_resp() call is busy modifying ep->resp and ep->arg when
+ * this function is invoked, the first spin_lock_bh() call in this function
+ * will wait until fc_seq_set_resp() has finished modifying these variables.
+ *
+ * Since fc_exch_done() invokes fc_seq_set_resp() it is guaranteed that that
+ * ep->resp() won't be invoked after fc_exch_done() has returned.
+ *
+ * The response handler itself may invoke fc_exch_done(), which will clear the
+ * ep->resp pointer.
+ *
+ * Return value:
+ * Returns true if and only if ep->resp has been invoked.
+ */
+static bool fc_invoke_resp(struct fc_exch *ep, struct fc_seq *sp,
+			   struct fc_frame *fp)
+{
+	void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg);
+	void *arg;
+	bool res = false;
+
+	spin_lock_bh(&ep->ex_lock);
+	ep->resp_active++;
+	if (ep->resp_task != current)
+		ep->resp_task = !ep->resp_task ? current : NULL;
+	resp = ep->resp;
+	arg = ep->arg;
+	spin_unlock_bh(&ep->ex_lock);
+
+	if (resp) {
+		resp(sp, fp, arg);
+		res = true;
+	} else if (!IS_ERR(fp)) {
+		fc_frame_free(fp);
+	}
+
+	spin_lock_bh(&ep->ex_lock);
+	if (--ep->resp_active == 0)
+		ep->resp_task = NULL;
+	spin_unlock_bh(&ep->ex_lock);
+
+	if (ep->resp_active == 0)
+		wake_up(&ep->resp_wq);
+
+	return res;
+}
+
+/**
  * fc_exch_timeout() - Handle exchange timer expiration
  * @work: The work_struct identifying the exchange that timed out
  */
@@ -689,8 +757,6 @@ static void fc_exch_timeout(struct work_struct *work)
 	struct fc_exch *ep = container_of(work, struct fc_exch,
 					  timeout_work.work);
 	struct fc_seq *sp = &ep->seq;
-	void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg);
-	void *arg;
 	u32 e_stat;
 	int rc = 1;
 
@@ -708,16 +774,13 @@ static void fc_exch_timeout(struct work_struct *work)
 			fc_exch_rrq(ep);
 		goto done;
 	} else {
-		resp = ep->resp;
-		arg = ep->arg;
-		ep->resp = NULL;
 		if (e_stat & ESB_ST_ABNORMAL)
 			rc = fc_exch_done_locked(ep);
 		spin_unlock_bh(&ep->ex_lock);
 		if (!rc)
 			fc_exch_delete(ep);
-		if (resp)
-			resp(sp, ERR_PTR(-FC_EX_TIMEOUT), arg);
+		fc_invoke_resp(ep, sp, ERR_PTR(-FC_EX_TIMEOUT));
+		fc_seq_set_resp(sp, NULL, ep->arg);
 		fc_seq_exch_abort(sp, 2 * ep->r_a_tov);
 		goto done;
 	}
@@ -804,6 +867,8 @@ hit:
 	ep->f_ctl = FC_FC_FIRST_SEQ;	/* next seq is first seq */
 	ep->rxid = FC_XID_UNKNOWN;
 	ep->class = mp->class;
+	ep->resp_active = 0;
+	init_waitqueue_head(&ep->resp_wq);
 	INIT_DELAYED_WORK(&ep->timeout_work, fc_exch_timeout);
 out:
 	return ep;
@@ -864,6 +929,8 @@ static struct fc_exch *fc_exch_find(struct fc_exch_mgr *mp, u16 xid)
  * fc_exch_done() - Indicate that an exchange/sequence tuple is complete and
  *		    the memory allocated for the related objects may be freed.
  * @sp: The sequence that has completed
+ *
+ * Note: May sleep if invoked from outside a response handler.
  */
 static void fc_exch_done(struct fc_seq *sp)
 {
@@ -873,6 +940,8 @@ static void fc_exch_done(struct fc_seq *sp)
 	spin_lock_bh(&ep->ex_lock);
 	rc = fc_exch_done_locked(ep);
 	spin_unlock_bh(&ep->ex_lock);
+
+	fc_seq_set_resp(sp, NULL, ep->arg);
 	if (!rc)
 		fc_exch_delete(ep);
 }
@@ -1436,9 +1505,7 @@ static void fc_exch_recv_req(struct fc_lport *lport, struct fc_exch_mgr *mp,
 		 * If new exch resp handler is valid then call that
 		 * first.
 		 */
-		if (ep->resp)
-			ep->resp(sp, fp, ep->arg);
-		else
+		if (!fc_invoke_resp(ep, sp, fp))
 			lport->tt.lport_recv(lport, fp);
 		fc_exch_release(ep);	/* release from lookup */
 	} else {
@@ -1462,8 +1529,6 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 	struct fc_exch *ep;
 	enum fc_sof sof;
 	u32 f_ctl;
-	void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg);
-	void *ex_resp_arg;
 	int rc;
 
 	ep = fc_exch_find(mp, ntohs(fh->fh_ox_id));
@@ -1506,14 +1571,11 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 
 	if (fc_sof_needs_ack(sof))
 		fc_seq_send_ack(sp, fp);
-	resp = ep->resp;
-	ex_resp_arg = ep->arg;
 
 	if (fh->fh_type != FC_TYPE_FCP && fr_eof(fp) == FC_EOF_T &&
 	    (f_ctl & (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) ==
 	    (FC_FC_LAST_SEQ | FC_FC_END_SEQ)) {
 		spin_lock_bh(&ep->ex_lock);
-		resp = ep->resp;
 		rc = fc_exch_done_locked(ep);
 		WARN_ON(fc_seq_exch(sp) != ep);
 		spin_unlock_bh(&ep->ex_lock);
@@ -1534,10 +1596,8 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
 	 * If new exch resp handler is valid then call that
 	 * first.
 	 */
-	if (resp)
-		resp(sp, fp, ex_resp_arg);
-	else
-		fc_frame_free(fp);
+	fc_invoke_resp(ep, sp, fp);
+
 	fc_exch_release(ep);
 	return;
 rel:
@@ -1576,8 +1636,6 @@ static void fc_exch_recv_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
  */
 static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp)
 {
-	void (*resp)(struct fc_seq *, struct fc_frame *fp, void *arg);
-	void *ex_resp_arg;
 	struct fc_frame_header *fh;
 	struct fc_ba_acc *ap;
 	struct fc_seq *sp;
@@ -1622,9 +1680,6 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp)
 		break;
 	}
 
-	resp = ep->resp;
-	ex_resp_arg = ep->arg;
-
 	/* do we need to do some other checks here. Can we reuse more of
 	 * fc_exch_recv_seq_resp
 	 */
@@ -1636,17 +1691,14 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp)
 	    ntoh24(fh->fh_f_ctl) & FC_FC_LAST_SEQ)
 		rc = fc_exch_done_locked(ep);
 	spin_unlock_bh(&ep->ex_lock);
+
+	fc_exch_hold(ep);
 	if (!rc)
 		fc_exch_delete(ep);
-
-	if (resp)
-		resp(sp, fp, ex_resp_arg);
-	else
-		fc_frame_free(fp);
-
+	fc_invoke_resp(ep, sp, fp);
 	if (has_rec)
 		fc_exch_timer_set(ep, ep->r_a_tov);
-
+	fc_exch_release(ep);
 }
 
 /**
@@ -1768,32 +1820,33 @@ static void fc_seq_ls_rjt(struct fc_frame *rx_fp, enum fc_els_rjt_reason reason,
 /**
  * fc_exch_reset() - Reset an exchange
  * @ep: The exchange to be reset
+ *
+ * Note: May sleep if invoked from outside a response handler.
  */
 static void fc_exch_reset(struct fc_exch *ep)
 {
 	struct fc_seq *sp;
-	void (*resp)(struct fc_seq *, struct fc_frame *, void *);
-	void *arg;
 	int rc = 1;
 
 	spin_lock_bh(&ep->ex_lock);
 	fc_exch_abort_locked(ep, 0);
 	ep->state |= FC_EX_RST_CLEANUP;
 	fc_exch_timer_cancel(ep);
-	resp = ep->resp;
-	ep->resp = NULL;
 	if (ep->esb_stat & ESB_ST_REC_QUAL)
 		atomic_dec(&ep->ex_refcnt);	/* drop hold for rec_qual */
 	ep->esb_stat &= ~ESB_ST_REC_QUAL;
-	arg = ep->arg;
 	sp = &ep->seq;
 	rc = fc_exch_done_locked(ep);
 	spin_unlock_bh(&ep->ex_lock);
+
+	fc_exch_hold(ep);
+
 	if (!rc)
 		fc_exch_delete(ep);
 
-	if (resp)
-		resp(sp, ERR_PTR(-FC_EX_CLOSED), arg);
+	fc_invoke_resp(ep, sp, ERR_PTR(-FC_EX_CLOSED));
+	fc_seq_set_resp(sp, NULL, ep->arg);
+	fc_exch_release(ep);
 }
 
 /**
diff --git a/include/scsi/libfc.h b/include/scsi/libfc.h
index e1379b4..52beadf 100644
--- a/include/scsi/libfc.h
+++ b/include/scsi/libfc.h
@@ -410,6 +410,12 @@ struct fc_seq {
  * @fh_type:      The frame type
  * @class:        The class of service
  * @seq:          The sequence in use on this exchange
+ * @resp_active:  Number of tasks that are concurrently executing @resp().
+ * @resp_task:    If @resp_active > 0, either the task executing @resp(), the
+ *                task that has been interrupted to execute the soft-IRQ
+ *                executing @resp() or NULL if more than one task is executing
+ *                @resp concurrently.
+ * @resp_wq:      Waitqueue for the tasks waiting on @resp_active.
  * @resp:         Callback for responses on this exchange
  * @destructor:   Called when destroying the exchange
  * @arg:          Passed as a void pointer to the resp() callback
@@ -441,6 +447,9 @@ struct fc_exch {
 	u32		    r_a_tov;
 	u32		    f_ctl;
 	struct fc_seq       seq;
+	int		    resp_active;
+	struct task_struct  *resp_task;
+	wait_queue_head_t   resp_wq;
 	void		    (*resp)(struct fc_seq *, struct fc_frame *, void *);
 	void		    *arg;
 	void		    (*destructor)(struct fc_seq *, void *);

[PATCH 13/19] fcp: Do not interpret check condition as underrun

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

This patch avoids that the FCoE initiator sends a REC message after
having received a SCSI response with non-zero status and non-zero
DATA IN buffer length.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/libfc/fc_fcp.c |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 5fd0f1f..1d7e76e 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -902,7 +902,8 @@ static void fc_fcp_resp(struct fc_fcp_pkt *fsp, struct fc_frame *fp)
 	/*
 	 * Check for missing or extra data frames.
 	 */
-	if (unlikely(fsp->xfer_len != expected_len)) {
+	if (unlikely(fsp->cdb_status == SAM_STAT_GOOD &&
+		     fsp->xfer_len != expected_len)) {
 		if (fsp->xfer_len < expected_len) {
 			/*
 			 * Some data may be queued locally,
@@ -955,12 +956,11 @@ static void fc_fcp_complete_locked(struct fc_fcp_pkt *fsp)
 		 * Test for transport underrun, independent of response
 		 * underrun status.
 		 */
-		if (fsp->xfer_len < fsp->data_len && !fsp->io_status &&
+		if (fsp->cdb_status == SAM_STAT_GOOD &&
+		    fsp->xfer_len < fsp->data_len && !fsp->io_status &&
 		    (!(fsp->scsi_comp_flags & FCP_RESID_UNDER) ||
-		     fsp->xfer_len < fsp->data_len - fsp->scsi_resid)) {
+		     fsp->xfer_len < fsp->data_len - fsp->scsi_resid))
 			fsp->status_code = FC_DATA_UNDRUN;
-			fsp->io_status = 0;
-		}
 	}
 
 	seq = fsp->seq_ptr;

[PATCH 14/19] fcoe: Declare fcoe_ctlr_mode_set() static

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

The function fcoe_ctlr_mode_set() is local, hence declare it static.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 35b1fb7..9e83a79 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -2828,8 +2828,8 @@ unlock:
  * disabled, so that should ensure that this routine is only called
  * when nothing is happening.
  */
-void fcoe_ctlr_mode_set(struct fc_lport *lport, struct fcoe_ctlr *fip,
-			enum fip_state fip_mode)
+static void fcoe_ctlr_mode_set(struct fc_lport *lport, struct fcoe_ctlr *fip,
+			       enum fip_state fip_mode)
 {
 	void *priv;
 

[PATCH 15/19] fcoe: Add missing newlines in debug messages

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

FCoE debug statements must end in a newline. Add one where it is missing.

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe.c       |   12 ++++++------
 drivers/scsi/fcoe/fcoe_sysfs.c |   12 ++++++------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 134ca3b..dff40b2 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1440,14 +1440,14 @@ static int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 	ctlr = fcoe_to_ctlr(fcoe);
 	lport = ctlr->lp;
 	if (unlikely(!lport)) {
-		FCOE_NETDEV_DBG(netdev, "Cannot find hba structure");
+		FCOE_NETDEV_DBG(netdev, "Cannot find hba structure\n");
 		goto err2;
 	}
 	if (!lport->link_up)
 		goto err2;
 
-	FCOE_NETDEV_DBG(netdev, "skb_info: len:%d data_len:%d head:%p "
-			"data:%p tail:%p end:%p sum:%d dev:%s",
+	FCOE_NETDEV_DBG(netdev,
+			"skb_info: len:%d data_len:%d head:%p data:%p tail:%p end:%p sum:%d dev:%s\n",
 			skb->len, skb->data_len, skb->head, skb->data,
 			skb_tail_pointer(skb), skb_end_pointer(skb),
 			skb->csum, skb->dev ? skb->dev->name : "<NULL>");
@@ -1794,13 +1794,13 @@ static void fcoe_recv_frame(struct sk_buff *skb)
 	lport = fr->fr_dev;
 	if (unlikely(!lport)) {
 		if (skb->destructor != fcoe_percpu_flush_done)
-			FCOE_NETDEV_DBG(skb->dev, "NULL lport in skb");
+			FCOE_NETDEV_DBG(skb->dev, "NULL lport in skb\n");
 		kfree_skb(skb);
 		return;
 	}
 
-	FCOE_NETDEV_DBG(skb->dev, "skb_info: len:%d data_len:%d "
-			"head:%p data:%p tail:%p end:%p sum:%d dev:%s",
+	FCOE_NETDEV_DBG(skb->dev,
+			"skb_info: len:%d data_len:%d head:%p data:%p tail:%p end:%p sum:%d dev:%s\n",
 			skb->len, skb->data_len,
 			skb->head, skb->data, skb_tail_pointer(skb),
 			skb_end_pointer(skb), skb->csum,
diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index c9382d6..922c9de 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -300,29 +300,29 @@ static ssize_t store_ctlr_mode(struct device *dev,
 
 	switch (ctlr->enabled) {
 	case FCOE_CTLR_ENABLED:
-		LIBFCOE_SYSFS_DBG(ctlr, "Cannot change mode when enabled.");
+		LIBFCOE_SYSFS_DBG(ctlr, "Cannot change mode when enabled.\n");
 		return -EBUSY;
 	case FCOE_CTLR_DISABLED:
 		if (!ctlr->f->set_fcoe_ctlr_mode) {
 			LIBFCOE_SYSFS_DBG(ctlr,
-					  "Mode change not supported by LLD.");
+					  "Mode change not supported by LLD.\n");
 			return -ENOTSUPP;
 		}
 
 		ctlr->mode = fcoe_parse_mode(mode);
 		if (ctlr->mode == FIP_CONN_TYPE_UNKNOWN) {
-			LIBFCOE_SYSFS_DBG(ctlr,
-					  "Unknown mode %s provided.", buf);
+			LIBFCOE_SYSFS_DBG(ctlr, "Unknown mode %s provided.\n",
+					  buf);
 			return -EINVAL;
 		}
 
 		ctlr->f->set_fcoe_ctlr_mode(ctlr);
-		LIBFCOE_SYSFS_DBG(ctlr, "Mode changed to %s.", buf);
+		LIBFCOE_SYSFS_DBG(ctlr, "Mode changed to %s.\n", buf);
 
 		return count;
 	case FCOE_CTLR_UNUSED:
 	default:
-		LIBFCOE_SYSFS_DBG(ctlr, "Mode change not supported.");
+		LIBFCOE_SYSFS_DBG(ctlr, "Mode change not supported.\n");
 		return -ENOTSUPP;
 	};
 }

[PATCH 16/19] fcoe: Reduce fcoe_sysfs_fcf_add() stack usage

[Robert Love]

From: Bart Van Assche <bvanassche@acm.org>

This patch fixes the following compiler warning:

drivers/scsi/fcoe/fcoe_ctlr.c: In function fcoe_sysfs_fcf_add:
drivers/scsi/fcoe/fcoe_ctlr.c:211:1: warning: the frame size of 1480 bytes is larger than 1024 bytes [-Wframe-larger-than=]

Signed-off-by: Bart Van Assche <bvanassche@acm.org>
Cc: Neil Horman <nhorman@tuxdriver.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c |   40 +++++++++++++++++++++++-----------------
 1 file changed, 23 insertions(+), 17 deletions(-)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 9e83a79..692c653 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -164,28 +164,30 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 {
 	struct fcoe_ctlr *fip = new->fip;
 	struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
-	struct fcoe_fcf_device temp, *fcf_dev;
-	int rc = 0;
+	struct fcoe_fcf_device *temp, *fcf_dev;
+	int rc = -ENOMEM;
 
 	LIBFCOE_FIP_DBG(fip, "New FCF fab %16.16llx mac %pM\n",
 			new->fabric_name, new->fcf_mac);
 
+	temp = kzalloc(sizeof(*temp), GFP_KERNEL);
+	if (!temp)
+		goto out;
+
 	mutex_lock(&ctlr_dev->lock);
 
-	temp.fabric_name = new->fabric_name;
-	temp.switch_name = new->switch_name;
-	temp.fc_map = new->fc_map;
-	temp.vfid = new->vfid;
-	memcpy(temp.mac, new->fcf_mac, ETH_ALEN);
-	temp.priority = new->pri;
-	temp.fka_period = new->fka_period;
-	temp.selected = 0; /* default to unselected */
-
-	fcf_dev = fcoe_fcf_device_add(ctlr_dev, &temp);
-	if (unlikely(!fcf_dev)) {
-		rc = -ENOMEM;
-		goto out;
-	}
+	temp->fabric_name = new->fabric_name;
+	temp->switch_name = new->switch_name;
+	temp->fc_map = new->fc_map;
+	temp->vfid = new->vfid;
+	memcpy(temp->mac, new->fcf_mac, ETH_ALEN);
+	temp->priority = new->pri;
+	temp->fka_period = new->fka_period;
+	temp->selected = 0; /* default to unselected */
+
+	fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
+	if (unlikely(!fcf_dev))
+		goto unlock;
 
 	/*
 	 * The fcoe_sysfs layer can return a CONNECTED fcf that
@@ -204,9 +206,13 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 
 	list_add(&new->list, &fip->fcfs);
 	fip->fcf_count++;
+	rc = 0;
 
-out:
+unlock:
 	mutex_unlock(&ctlr_dev->lock);
+
+out:
+	kfree(temp);
 	return rc;
 }
 

[PATCH 17/19] libfcoe: Make fcoe_sysfs optional / fix fnic NULL exception

[Robert Love]

fnic doesn't use any of the create/destroy/enable/disable interfaces
either from the (legacy) module paramaters or the (new) fcoe_sysfs
interfaces. When fcoe_sysfs was introduced fnic wasn't changed since
it wasn't using the interfaces. libfcoe incorrectly assumed that that
all of its users were using fcoe_sysfs and when adding and deleting
FCFs would assume the existance of a fcoe_ctlr_device. fnic was not
allocating this structure because it doesn't care about the standard
user interfaces (fnic starts on link only). If/When libfcoe tried to use
the fcoe_ctlr_device's lock for the first time a NULL pointer exception
would be triggered.

Since fnic doesn't care about sysfs or user interfaces, the solution
is to drop libfcoe's assumption that all drivers are using fcoe_sysfs.

This patch accomplishes this by changing some of the structure
relationships.

We need a way to determine when a LLD is using fcoe_sysfs or not and
we can do that by checking for the existance of the fcoe_ctlr_device.
Prior to this patch, it was assumed that the fcoe_ctlr structure was
allocated with the fcoe_ctlr_device and immediately followed it in
memory. To reach the fcoe_ctlr_device we would simply go back in memory
from the fcoe_ctlr to get the fcoe_ctlr_device.

Since fnic doesn't allocate the fcoe_ctlr_device, we cannot keep that
assumption. This patch adds a pointer from the fcoe_ctlr to the
fcoe_ctlr_device. For bnx2fc and fcoe we will continue to allocate the
two structures together, but then we'll set the ctlr->cdev pointer
to point at the fcoe_ctlr_device. fnic will not change and will continue
to allocate the fcoe_ctlr itself, and ctlr->cdev will remain NULL.

When libfcoe adds fcoe_fcf's to the fcoe_ctlr it will check if ctlr->cdev
is set and only if so will it continue to interact with fcoe_sysfs.

Signed-off-by: Robert Love <robert.w.love@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Tested-by: Hiral Patel <hiralpat@cisco.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    1 
 drivers/scsi/fcoe/fcoe.c          |    1 
 drivers/scsi/fcoe/fcoe_ctlr.c     |   94 +++++++++++++++++++++++++------------
 include/scsi/libfcoe.h            |    7 ++-
 4 files changed, 71 insertions(+), 32 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 69ac554..27f2cc4 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -1381,6 +1381,7 @@ struct bnx2fc_interface *bnx2fc_interface_create(struct bnx2fc_hba *hba,
 		return NULL;
 	}
 	ctlr = fcoe_ctlr_device_priv(ctlr_dev);
+	ctlr->cdev = ctlr_dev;
 	interface = fcoe_ctlr_priv(ctlr);
 	dev_hold(netdev);
 	kref_init(&interface->kref);
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index dff40b2..8626988 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -408,6 +408,7 @@ static struct fcoe_interface *fcoe_interface_create(struct net_device *netdev,
 	}
 
 	ctlr = fcoe_ctlr_device_priv(ctlr_dev);
+	ctlr->cdev = ctlr_dev;
 	fcoe = fcoe_ctlr_priv(ctlr);
 
 	dev_hold(netdev);
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 692c653..75efdbc 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -160,10 +160,16 @@ void fcoe_ctlr_init(struct fcoe_ctlr *fip, enum fip_state mode)
 }
 EXPORT_SYMBOL(fcoe_ctlr_init);
 
+/**
+ * fcoe_sysfs_fcf_add() - Add a fcoe_fcf{,_device} to a fcoe_ctlr{,_device}
+ * @new: The newly discovered FCF
+ *
+ * Called with fip->ctlr_mutex held
+ */
 static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 {
 	struct fcoe_ctlr *fip = new->fip;
-	struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+	struct fcoe_ctlr_device *ctlr_dev;
 	struct fcoe_fcf_device *temp, *fcf_dev;
 	int rc = -ENOMEM;
 
@@ -174,8 +180,6 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 	if (!temp)
 		goto out;
 
-	mutex_lock(&ctlr_dev->lock);
-
 	temp->fabric_name = new->fabric_name;
 	temp->switch_name = new->switch_name;
 	temp->fc_map = new->fc_map;
@@ -185,55 +189,83 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 	temp->fka_period = new->fka_period;
 	temp->selected = 0; /* default to unselected */
 
-	fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
-	if (unlikely(!fcf_dev))
-		goto unlock;
-
 	/*
-	 * The fcoe_sysfs layer can return a CONNECTED fcf that
-	 * has a priv (fcf was never deleted) or a CONNECTED fcf
-	 * that doesn't have a priv (fcf was deleted). However,
-	 * libfcoe will always delete FCFs before trying to add
-	 * them. This is ensured because both recv_adv and
-	 * age_fcfs are protected by the the fcoe_ctlr's mutex.
-	 * This means that we should never get a FCF with a
-	 * non-NULL priv pointer.
+	 * If ctlr_dev doesn't exist then it means we're a libfcoe user
+	 * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device.
+	 * fnic would be an example of a driver with this behavior. In this
+	 * case we want to add the fcoe_fcf to the fcoe_ctlr list, but we
+	 * don't want to make sysfs changes.
 	 */
-	BUG_ON(fcf_dev->priv);
 
-	fcf_dev->priv = new;
-	new->fcf_dev = fcf_dev;
+	ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+	if (ctlr_dev) {
+		mutex_lock(&ctlr_dev->lock);
+		fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
+		if (unlikely(!fcf_dev)) {
+			rc = -ENOMEM;
+			goto out;
+		}
+
+		/*
+		 * The fcoe_sysfs layer can return a CONNECTED fcf that
+		 * has a priv (fcf was never deleted) or a CONNECTED fcf
+		 * that doesn't have a priv (fcf was deleted). However,
+		 * libfcoe will always delete FCFs before trying to add
+		 * them. This is ensured because both recv_adv and
+		 * age_fcfs are protected by the the fcoe_ctlr's mutex.
+		 * This means that we should never get a FCF with a
+		 * non-NULL priv pointer.
+		 */
+		BUG_ON(fcf_dev->priv);
+
+		fcf_dev->priv = new;
+		new->fcf_dev = fcf_dev;
+		mutex_unlock(&ctlr_dev->lock);
+	}
 
 	list_add(&new->list, &fip->fcfs);
 	fip->fcf_count++;
 	rc = 0;
 
-unlock:
-	mutex_unlock(&ctlr_dev->lock);
-
 out:
 	kfree(temp);
 	return rc;
 }
 
+/**
+ * fcoe_sysfs_fcf_del() - Remove a fcoe_fcf{,_device} to a fcoe_ctlr{,_device}
+ * @new: The FCF to be removed
+ *
+ * Called with fip->ctlr_mutex held
+ */
 static void fcoe_sysfs_fcf_del(struct fcoe_fcf *new)
 {
 	struct fcoe_ctlr *fip = new->fip;
-	struct fcoe_ctlr_device *ctlr_dev = fcoe_ctlr_to_ctlr_dev(fip);
+	struct fcoe_ctlr_device *cdev;
 	struct fcoe_fcf_device *fcf_dev;
 
 	list_del(&new->list);
 	fip->fcf_count--;
 
-	mutex_lock(&ctlr_dev->lock);
-
-	fcf_dev = fcoe_fcf_to_fcf_dev(new);
-	WARN_ON(!fcf_dev);
-	new->fcf_dev = NULL;
-	fcoe_fcf_device_delete(fcf_dev);
-	kfree(new);
-
-	mutex_unlock(&ctlr_dev->lock);
+	/*
+	 * If ctlr_dev doesn't exist then it means we're a libfcoe user
+	 * who doesn't use fcoe_syfs and didn't allocate a fcoe_ctlr_device
+	 * or a fcoe_fcf_device.
+	 *
+	 * fnic would be an example of a driver with this behavior. In this
+	 * case we want to remove the fcoe_fcf from the fcoe_ctlr list (above),
+	 * but we don't want to make sysfs changes.
+	 */
+	cdev = fcoe_ctlr_to_ctlr_dev(fip);
+	if (cdev) {
+		mutex_lock(&cdev->lock);
+		fcf_dev = fcoe_fcf_to_fcf_dev(new);
+		WARN_ON(!fcf_dev);
+		new->fcf_dev = NULL;
+		fcoe_fcf_device_delete(fcf_dev);
+		kfree(new);
+		mutex_unlock(&cdev->lock);
+	}
 }
 
 /**
diff --git a/include/scsi/libfcoe.h b/include/scsi/libfcoe.h
index 4427393..de7e3ee 100644
--- a/include/scsi/libfcoe.h
+++ b/include/scsi/libfcoe.h
@@ -90,6 +90,7 @@ enum fip_state {
  * @lp:		   &fc_lport: libfc local port.
  * @sel_fcf:	   currently selected FCF, or NULL.
  * @fcfs:	   list of discovered FCFs.
+ * @cdev:          (Optional) pointer to sysfs fcoe_ctlr_device.
  * @fcf_count:	   number of discovered FCF entries.
  * @sol_time:	   time when a multicast solicitation was last sent.
  * @sel_time:	   time after which to select an FCF.
@@ -127,6 +128,7 @@ struct fcoe_ctlr {
 	struct fc_lport *lp;
 	struct fcoe_fcf *sel_fcf;
 	struct list_head fcfs;
+	struct fcoe_ctlr_device *cdev;
 	u16 fcf_count;
 	unsigned long sol_time;
 	unsigned long sel_time;
@@ -168,8 +170,11 @@ static inline void *fcoe_ctlr_priv(const struct fcoe_ctlr *ctlr)
 	return (void *)(ctlr + 1);
 }
 
+/*
+ * This assumes that the fcoe_ctlr (x) is allocated with the fcoe_ctlr_device.
+ */
 #define fcoe_ctlr_to_ctlr_dev(x)					\
-	(struct fcoe_ctlr_device *)(((struct fcoe_ctlr_device *)(x)) - 1)
+	(x)->cdev
 
 /**
  * struct fcoe_fcf - Fibre-Channel Forwarder

[PATCH 18/19] fcoe: Fix missing mutex_unlock in fcoe_sysfs_fcf_add error path

[Robert Love]

From: Neil Horman <nhorman@tuxdriver.com>

In this pending patch:
http://patchwork.open-fcoe.org/patch/104/

Tomas Henzl noted that the error path when fcoe_fcf_device_add fails, was
missing a mutex_unlock call.

Not sure what staet the integration of the above patch is in, but if you could
either merge this with it, or apply it on top of what you already have, that
would be great.  Thanks!

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: thenzl@redhat.com
Reported-by: thenzl@redhat.com
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/fcoe/fcoe_ctlr.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 75efdbc..2aba32f 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -203,6 +203,7 @@ static int fcoe_sysfs_fcf_add(struct fcoe_fcf *new)
 		fcf_dev = fcoe_fcf_device_add(ctlr_dev, temp);
 		if (unlikely(!fcf_dev)) {
 			rc = -ENOMEM;
+			mutex_unlock(&ctlr_dev->lock);
 			goto out;
 		}
 

[PATCH 19/19] scsi: Convert uses of compare_ether_addr to ether_addr_equal

[Robert Love]

From: Joe Perches <joe@perches.com>

Preliminary to removing compare_ether_addr altogether:

Use the new bool function ether_addr_equal to add
some clarity and reduce the likelihood for misuse
of compare_ether_addr for sorting.

Done via cocci script:

$ cat compare_ether_addr.cocci
@@
expression a,b;
@@
-	!compare_ether_addr(a, b)
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	compare_ether_addr(a, b)
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) == 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!ether_addr_equal(a, b) != 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) == 0
+	!ether_addr_equal(a, b)

@@
expression a,b;
@@
-	ether_addr_equal(a, b) != 0
+	ether_addr_equal(a, b)

@@
expression a,b;
@@
-	!!ether_addr_equal(a, b)
+	ether_addr_equal(a, b)

Signed-off-by: Joe Perches <joe@perches.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
---
 drivers/scsi/bnx2fc/bnx2fc_fcoe.c |    3 +--
 drivers/scsi/fcoe/fcoe.c          |    2 +-
 drivers/scsi/fcoe/fcoe_ctlr.c     |   22 +++++++++++-----------
 drivers/scsi/fcoe/fcoe_sysfs.c    |    2 +-
 drivers/scsi/fnic/fnic_fcs.c      |    6 +++---
 5 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
index 27f2cc4..23f6eea 100644
--- a/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
+++ b/drivers/scsi/bnx2fc/bnx2fc_fcoe.c
@@ -542,8 +542,7 @@ static void bnx2fc_recv_frame(struct sk_buff *skb)
 	vn_port = fc_vport_id_lookup(lport, ntoh24(fh->fh_d_id));
 	if (vn_port) {
 		port = lport_priv(vn_port);
-		if (compare_ether_addr(port->data_src_addr, dest_mac)
-		    != 0) {
+		if (!ether_addr_equal(port->data_src_addr, dest_mac)) {
 			BNX2FC_HBA_DBG(lport, "fpma mismatch\n");
 			put_cpu();
 			kfree_skb(skb);
diff --git a/drivers/scsi/fcoe/fcoe.c b/drivers/scsi/fcoe/fcoe.c
index 8626988..f317000 100644
--- a/drivers/scsi/fcoe/fcoe.c
+++ b/drivers/scsi/fcoe/fcoe.c
@@ -1462,7 +1462,7 @@ static int fcoe_rcv(struct sk_buff *skb, struct net_device *netdev,
 	eh = eth_hdr(skb);
 
 	if (is_fip_mode(ctlr) &&
-	    compare_ether_addr(eh->h_source, ctlr->dest_addr)) {
+	    !ether_addr_equal(eh->h_source, ctlr->dest_addr)) {
 		FCOE_NETDEV_DBG(netdev, "wrong source mac address:%pM\n",
 				eh->h_source);
 		goto err;
diff --git a/drivers/scsi/fcoe/fcoe_ctlr.c b/drivers/scsi/fcoe/fcoe_ctlr.c
index 2aba32f..34a1b1f 100644
--- a/drivers/scsi/fcoe/fcoe_ctlr.c
+++ b/drivers/scsi/fcoe/fcoe_ctlr.c
@@ -339,7 +339,7 @@ static void fcoe_ctlr_announce(struct fcoe_ctlr *fip)
 	spin_unlock_bh(&fip->ctlr_lock);
 	sel = fip->sel_fcf;
 
-	if (sel && !compare_ether_addr(sel->fcf_mac, fip->dest_addr))
+	if (sel && ether_addr_equal(sel->fcf_mac, fip->dest_addr))
 		goto unlock;
 	if (!is_zero_ether_addr(fip->dest_addr)) {
 		printk(KERN_NOTICE "libfcoe: host%d: "
@@ -1039,7 +1039,7 @@ static void fcoe_ctlr_recv_adv(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		if (fcf->switch_name == new.switch_name &&
 		    fcf->fabric_name == new.fabric_name &&
 		    fcf->fc_map == new.fc_map &&
-		    compare_ether_addr(fcf->fcf_mac, new.fcf_mac) == 0) {
+		    ether_addr_equal(fcf->fcf_mac, new.fcf_mac)) {
 			found = 1;
 			break;
 		}
@@ -1379,7 +1379,7 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 			mp = (struct fip_mac_desc *)desc;
 			if (dlen < sizeof(*mp))
 				goto err;
-			if (compare_ether_addr(mp->fd_mac, fcf->fcf_mac))
+			if (!ether_addr_equal(mp->fd_mac, fcf->fcf_mac))
 				goto err;
 			desc_mask &= ~BIT(FIP_DT_MAC);
 			break;
@@ -1457,8 +1457,8 @@ static void fcoe_ctlr_recv_clr_vlink(struct fcoe_ctlr *fip,
 			 * 'port_id' is already validated, check MAC address and
 			 * wwpn
 			 */
-			if (compare_ether_addr(fip->get_src_addr(vn_port),
-						vp->fd_mac) != 0 ||
+			if (!ether_addr_equal(fip->get_src_addr(vn_port),
+					      vp->fd_mac) ||
 				get_unaligned_be64(&vp->fd_wwpn) !=
 							vn_port->wwpn)
 				continue;
@@ -1521,12 +1521,12 @@ static int fcoe_ctlr_recv_handler(struct fcoe_ctlr *fip, struct sk_buff *skb)
 		goto drop;
 	eh = eth_hdr(skb);
 	if (fip->mode == FIP_MODE_VN2VN) {
-		if (compare_ether_addr(eh->h_dest, fip->ctl_src_addr) &&
-		    compare_ether_addr(eh->h_dest, fcoe_all_vn2vn) &&
-		    compare_ether_addr(eh->h_dest, fcoe_all_p2p))
+		if (!ether_addr_equal(eh->h_dest, fip->ctl_src_addr) &&
+		    !ether_addr_equal(eh->h_dest, fcoe_all_vn2vn) &&
+		    !ether_addr_equal(eh->h_dest, fcoe_all_p2p))
 			goto drop;
-	} else if (compare_ether_addr(eh->h_dest, fip->ctl_src_addr) &&
-		   compare_ether_addr(eh->h_dest, fcoe_all_enode))
+	} else if (!ether_addr_equal(eh->h_dest, fip->ctl_src_addr) &&
+		   !ether_addr_equal(eh->h_dest, fcoe_all_enode))
 		goto drop;
 	fiph = (struct fip_header *)skb->data;
 	op = ntohs(fiph->fip_op);
@@ -1898,7 +1898,7 @@ int fcoe_ctlr_recv_flogi(struct fcoe_ctlr *fip, struct fc_lport *lport,
 		 * address_mode flag to use FC_OUI-based Ethernet DA.
 		 * Otherwise we use the FCoE gateway addr
 		 */
-		if (!compare_ether_addr(sa, (u8[6])FC_FCOE_FLOGI_MAC)) {
+		if (ether_addr_equal(sa, (u8[6])FC_FCOE_FLOGI_MAC)) {
 			fcoe_ctlr_map_dest(fip);
 		} else {
 			memcpy(fip->dest_addr, sa, ETH_ALEN);
diff --git a/drivers/scsi/fcoe/fcoe_sysfs.c b/drivers/scsi/fcoe/fcoe_sysfs.c
index 922c9de..d0d9a2d 100644
--- a/drivers/scsi/fcoe/fcoe_sysfs.c
+++ b/drivers/scsi/fcoe/fcoe_sysfs.c
@@ -653,7 +653,7 @@ static int fcoe_fcf_device_match(struct fcoe_fcf_device *new,
 	if (new->switch_name == old->switch_name &&
 	    new->fabric_name == old->fabric_name &&
 	    new->fc_map == old->fc_map &&
-	    compare_ether_addr(new->mac, old->mac) == 0)
+	    ether_addr_equal(new->mac, old->mac))
 		return 1;
 	return 0;
 }
diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index 006fa92..902520c 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -651,13 +651,13 @@ void fnic_update_mac_locked(struct fnic *fnic, u8 *new)
 
 	if (is_zero_ether_addr(new))
 		new = ctl;
-	if (!compare_ether_addr(data, new))
+	if (ether_addr_equal(data, new))
 		return;
 	FNIC_FCS_DBG(KERN_DEBUG, fnic->lport->host, "update_mac %pM\n", new);
-	if (!is_zero_ether_addr(data) && compare_ether_addr(data, ctl))
+	if (!is_zero_ether_addr(data) && !ether_addr_equal(data, ctl))
 		vnic_dev_del_addr(fnic->vdev, data);
 	memcpy(data, new, ETH_ALEN);
-	if (compare_ether_addr(new, ctl))
+	if (!ether_addr_equal(new, ctl))
 		vnic_dev_add_addr(fnic->vdev, new);
 }