On 05/27/15 03:13, vasu.dev@xxxxxxxxxxxxxxx wrote:
> ...but question is why even we added schedule()
> which is now causing this side effect in this commit:-
>
> commit 7030fd626129ec4d616784516a462d317c251d39
> Author: Bart Van Assche<bvanassche@xxxxxxx>
> Date: Sat Aug 17 20:34:43 2013 +0000
>
> May be we should remove schedule() there which again does ep->ex_lock
> drop and re-grab logic for schedule. Do we really need schedule there
> added by above patch ?
Hello Vasu,
The layers above libfc need a way to wait until ongoing invocations of
their own response handler have finished. This is why the schedule() call
has been added in fc_seq_set_resp(). Regarding avoiding to drop and re-grab
the lock, how about replacing patch 4/4 by the patch below ?
Thanks,
Bart.
[PATCH] libfc: Fix the device reset implementation
Avoid 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@xxxxxxxxxxx>
Cc: Vasu Dev <vasu.dev@xxxxxxxxx>
Cc: stable <stable@xxxxxxxxxxxxxxx>
---
drivers/scsi/libfc/fc_exch.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
index 30f9ef0..4a1fe55 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -709,8 +709,8 @@ static int fc_seq_exch_abort(const struct fc_seq *req_sp,
* 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.
+ * The response handler itself may invoke fc_exch_done(), which will set
+ * FC_EX_DONE.
*
* Return value:
* Returns true if and only if ep->resp has been invoked.
@@ -730,7 +730,8 @@ static bool fc_invoke_resp(struct fc_exch *ep, struct fc_seq *sp,
arg = ep->arg;
spin_unlock_bh(&ep->ex_lock);
- if (resp) {
+ if (resp && (!(ep->state & (FC_EX_RST_CLEANUP | FC_EX_DONE)) ||
+ IS_ERR(fp))) {
resp(sp, fp, arg);
res = true;
}
@@ -939,7 +940,6 @@ static void fc_exch_done(struct fc_seq *sp)
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);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
