[PATCH 0/3] libfc patches for kernel v4.2

[PATCH 0/3] libfc patches for kernel v4.2

[Vasu Dev]

These libfc fixes are for kernel 4.2 and with two patches CCed for stable
kernel also, those two could be picked for current 4.1 cycle now but in case
too late for current 4.1 then anyway should get picked up by 4.1 stable
kernel later.

These patches were posted by Bart Van Assche at open-fcoe.org and then 
I updated them per discussion on original patches to pick revised patch
and add related comments at:

http://lists.open-fcoe.org/pipermail/fcoe-devel/2015-May/012467.html


Bart Van Assche (3):
      libfc: Fix fc_fcp_cleanup_each_cmd()
      libfc: Fix fc_exch_recv_req() error path
      libfc: Fix a typo in a source code comment


 drivers/scsi/libfc/fc_exch.c |    8 ++++----
 drivers/scsi/libfc/fc_fcp.c  |   21 ++++++++++++++++++---
 2 files changed, 22 insertions(+), 7 deletions(-)

-- 

//Vasu

[PATCH 1/3] libfc: Fix a typo in a source code comment

[Vasu Dev]

From: Bart Van Assche <bart.vanassche@sandisk.com>

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

 drivers/scsi/libfc/fc_fcp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index c679594..4cd49d4 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -191,7 +191,7 @@ static void fc_fcp_pkt_hold(struct fc_fcp_pkt *fsp)
 }
 
 /**
- * fc_fcp_pkt_destory() - Release hold on a fcp_pkt
+ * fc_fcp_pkt_destroy() - Release hold on a fcp_pkt
  * @seq: The sequence that the FCP packet is on (required by destructor API)
  * @fsp: The FCP packet to be released
  *

[PATCH 2/3] libfc: Fix fc_exch_recv_req() error path

[Vasu Dev]

From: Bart Van Assche <bart.vanassche@sandisk.com>

Due to patch "libfc: Do not invoke the response handler after
fc_exch_done()" (commit ID 7030fd62) the lport_recv() call
in fc_exch_recv_req() is passed a dangling pointer. Avoid this
by moving the fc_frame_free() call from fc_invoke_resp() to its
callers. This patch fixes the following crash:

general protection fault: 0000 [#3] PREEMPT SMP
RIP: fc_lport_recv_req+0x72/0x280 [libfc]
Call Trace:
 fc_exch_recv+0x642/0xde0 [libfc]
 fcoe_percpu_receive_thread+0x46a/0x5ed [fcoe]
 kthread+0x10a/0x120
 ret_from_fork+0x42/0x70

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: Vasu Dev <vasu.dev@intel.com>
Cc: stable <stable@vger.kernel.org>

-v2

Updated to revised patch posted at:
http://lists.open-fcoe.org/pipermail/fcoe-devel/2015-May/012462.html

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

 drivers/scsi/libfc/fc_exch.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 1b3a094..30f9ef0 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -733,8 +733,6 @@ static bool fc_invoke_resp(struct fc_exch *ep, struct fc_seq *sp,
 	if (resp) {
 		resp(sp, fp, arg);
 		res = true;
-	} else if (!IS_ERR(fp)) {
-		fc_frame_free(fp);
 	}
 
 	spin_lock_bh(&ep->ex_lock);
@@ -1596,7 +1594,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.
 	 */
-	fc_invoke_resp(ep, sp, fp);
+	if (!fc_invoke_resp(ep, sp, fp))
+		fc_frame_free(fp);
 
 	fc_exch_release(ep);
 	return;
@@ -1695,7 +1694,8 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp)
 	fc_exch_hold(ep);
 	if (!rc)
 		fc_exch_delete(ep);
-	fc_invoke_resp(ep, sp, fp);
+	if (!fc_invoke_resp(ep, sp, fp))
+		fc_frame_free(fp);
 	if (has_rec)
 		fc_exch_timer_set(ep, ep->r_a_tov);
 	fc_exch_release(ep);

[PATCH 3/3] libfc: Fix fc_fcp_cleanup_each_cmd()

[Vasu Dev]

From: Bart Van Assche <bart.vanassche@sandisk.com>

Since fc_fcp_cleanup_cmd() can sleep this function must not
be called while holding a spinlock. This patch avoids that
fc_fcp_cleanup_each_cmd() triggers the following bug:

BUG: scheduling while atomic: sg_reset/1512/0x00000202
1 lock held by sg_reset/1512:
 #0:  (&(&fsp->scsi_pkt_lock)->rlock){+.-...}, at: [<ffffffffc0225cd5>] fc_fcp_cleanup_each_cmd.isra.21+0xa5/0x150 [libfc]
Preemption disabled at:[<ffffffffc0225cd5>] fc_fcp_cleanup_each_cmd.isra.21+0xa5/0x150 [libfc]
Call Trace:
 [<ffffffff816c612c>] dump_stack+0x4f/0x7b
 [<ffffffff810828bc>] __schedule_bug+0x6c/0xd0
 [<ffffffff816c87aa>] __schedule+0x71a/0xa10
 [<ffffffff816c8ad2>] schedule+0x32/0x80
 [<ffffffffc0217eac>] fc_seq_set_resp+0xac/0x100 [libfc]
 [<ffffffffc0218b11>] fc_exch_done+0x41/0x60 [libfc]
 [<ffffffffc0225cff>] fc_fcp_cleanup_each_cmd.isra.21+0xcf/0x150 [libfc]
 [<ffffffffc0225f43>] fc_eh_device_reset+0x1c3/0x270 [libfc]
 [<ffffffff814a2cc9>] scsi_try_bus_device_reset+0x29/0x60
 [<ffffffff814a3908>] scsi_ioctl_reset+0x258/0x2d0
 [<ffffffff814a2650>] scsi_ioctl+0x150/0x440
 [<ffffffff814b3a9d>] sd_ioctl+0xad/0x120
 [<ffffffff8132f266>] blkdev_ioctl+0x1b6/0x810
 [<ffffffff811da608>] block_ioctl+0x38/0x40
 [<ffffffff811b4e08>] do_vfs_ioctl+0x2f8/0x530
 [<ffffffff811b50c1>] SyS_ioctl+0x81/0xa0
 [<ffffffff816cf8b2>] system_call_fastpath+0x16/0x7a

Signed-off-by: Bart Van Assche <bart.vanassche@sandisk.com>
Cc: stable <stable@vger.kernel.org>

[ Added TODO notes to fix by removing entire schedule() code flow
  as discussed here at:
  http://lists.open-fcoe.org/pipermail/fcoe-devel/2015-May/012471.html ]

Signed-off-by: Vasu Dev <vasu.dev@intel.com>
---

 drivers/scsi/libfc/fc_fcp.c |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/libfc/fc_fcp.c b/drivers/scsi/libfc/fc_fcp.c
index 4cd49d4..5121272 100644
--- a/drivers/scsi/libfc/fc_fcp.c
+++ b/drivers/scsi/libfc/fc_fcp.c
@@ -1039,11 +1039,26 @@ restart:
 		fc_fcp_pkt_hold(fsp);
 		spin_unlock_irqrestore(&si->scsi_queue_lock, flags);
 
-		if (!fc_fcp_lock_pkt(fsp)) {
+		spin_lock_bh(&fsp->scsi_pkt_lock);
+		if (!(fsp->state & FC_SRB_COMPL)) {
+			fsp->state |= FC_SRB_COMPL;
+			/*
+			 * TODO: dropping scsi_pkt_lock and then reacquiring
+			 * again around fc_fcp_cleanup_cmd() is required,
+			 * since fc_fcp_cleanup_cmd() calls into
+			 * fc_seq_set_resp() and that func preempts cpu using
+			 * schedule. May be schedule and related code should be
+			 * removed instead of unlocking here to avoid scheduling
+			 * while atomic bug.
+			 */
+			spin_unlock_bh(&fsp->scsi_pkt_lock);
+
 			fc_fcp_cleanup_cmd(fsp, error);
+
+			spin_lock_bh(&fsp->scsi_pkt_lock);
 			fc_io_compl(fsp);
-			fc_fcp_unlock_pkt(fsp);
 		}
+		spin_unlock_bh(&fsp->scsi_pkt_lock);
 
 		fc_fcp_pkt_release(fsp);
 		spin_lock_irqsave(&si->scsi_queue_lock, flags);

Re: [PATCH 0/3] libfc patches for kernel v4.2

[Vasu Dev]

On Fri, 2015-06-05 at 14:20 -0700, Vasu Dev wrote:
> These libfc fixes are for kernel 4.2 and with two patches CCed for stable
> kernel also, those two could be picked for current 4.1 cycle now but in case
> too late for current 4.1 then anyway should get picked up by 4.1 stable
> kernel later.
> 
> These patches were posted by Bart Van Assche at open-fcoe.org and then 
> I updated them per discussion on original patches to pick revised patch
> and add related comments at:
> 
> http://lists.open-fcoe.org/pipermail/fcoe-devel/2015-May/012467.html
> 
> 
> Bart Van Assche (3):
>       libfc: Fix fc_fcp_cleanup_each_cmd()
>       libfc: Fix fc_exch_recv_req() error path
>       libfc: Fix a typo in a source code comment
> 
> 
>  drivers/scsi/libfc/fc_exch.c |    8 ++++----
>  drivers/scsi/libfc/fc_fcp.c  |   21 ++++++++++++++++++---
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 

Adding Bart to CC list, I thought stg --auto would CC to original patch
author but that is not the case, sorry for another email for this.


Thanks,
Vasu

Re: [PATCH 0/3] libfc patches for kernel v4.2

[Vasu Dev]

On Fri, 2015-06-05 at 14:20 -0700, Vasu Dev wrote:
> These libfc fixes are for kernel 4.2 and with two patches CCed for stable
> kernel also, those two could be picked for current 4.1 cycle now but in case
> too late for current 4.1 then anyway should get picked up by 4.1 stable
> kernel later.
> 
> These patches were posted by Bart Van Assche at open-fcoe.org and then 
> I updated them per discussion on original patches to pick revised patch
> and add related comments at:
> 
> http://lists.open-fcoe.org/pipermail/fcoe-devel/2015-May/012467.html
> 
> 
> Bart Van Assche (3):
>       libfc: Fix fc_fcp_cleanup_each_cmd()
>       libfc: Fix fc_exch_recv_req() error path
>       libfc: Fix a typo in a source code comment
> 
> 
>  drivers/scsi/libfc/fc_exch.c |    8 ++++----
>  drivers/scsi/libfc/fc_fcp.c  |   21 ++++++++++++++++++---
>  2 files changed, 22 insertions(+), 7 deletions(-)
> 

Hello James and Christoph,

This patch series is not applied as of now, so can you apply the patch
series ?

Thanks,
Vasu