RE: [RFC PATCH] scsi_transport_fc: Make sure commands are completed when rport is offline

[Sarang Radke <sarang.radke@xxxxxxxxxx>]

James S./Fujita,

Thanks for the comments. The description of scenario was the one with which I noted the issue. But yes, the same would be true for any rport which is offline.

I am resending the patch with the suggested modifications.

-Thanks,
Sarang

>From 84a1f847c71382f67f97ea956b2283cf6d737340 Mon Sep 17 00:00:00 2001
From: Sarang Radke <sarang.radke@xxxxxxxxxx>
Date: Tue, 9 Mar 2010 12:20:29 -0500
Subject: [PATCH] Make sure commands are completed for offline rport

blk_end_request doesn't complete a bidi request
successfully

The unfinished request eventually triggers a panic in
timeout handling routine fc_bsg_job_timeout as
req->special is NULL

Use blk_end_request_all to end the request unconditionally

Signed-off-by: Lalit Chandivade <lalit.chandivade@xxxxxxxxxx>
---
 drivers/scsi/scsi_transport_fc.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/scsi_transport_fc.c
index 79660ee..5819457 100644
--- a/drivers/scsi/scsi_transport_fc.c
+++ b/drivers/scsi/scsi_transport_fc.c
@@ -3852,7 +3852,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
                if (rport && (rport->port_state != FC_PORTSTATE_ONLINE)) {
                        req->errors = -ENXIO;
                        spin_unlock_irq(q->queue_lock);
-                       blk_end_request(req, -ENXIO, blk_rq_bytes(req));
+                       blk_end_request_all(req, -ENXIO);
                        spin_lock_irq(q->queue_lock);
                        continue;
                }
@@ -3862,7 +3862,7 @@ fc_bsg_request_handler(struct request_queue *q, struct Scsi_Host *shost,
                ret = fc_req_to_bsgjob(shost, rport, req);
                if (ret) {
                        req->errors = ret;
-                       blk_end_request(req, ret, blk_rq_bytes(req));
+                       blk_end_request_all(req, ret);
                        spin_lock_irq(q->queue_lock);
                        continue;
                }
--
1.5.6


-----Original Message-----
From: James Smart [mailto:james.smart@xxxxxxxxxx] 
Sent: Tuesday, March 09, 2010 8:11 PM
To: Sarang Radke
Cc: fujita.tomonori@xxxxxxxxxxxxx; James.Bottomley@xxxxxxx; linux-scsi@xxxxxxxxxxxxxxx; Andrew Vasquez; Lalit Chandivade; Giridhar Malavali; Ravi Anand
Subject: Re: [RFC PATCH] scsi_transport_fc: Make sure commands are completed when rport is offline

I'm guessing your opening line describing the scenario wasn't correct. To 
match the patch, the request had to be issued when the rport is offline.
True ?

I think the change is fine, but the same change needs to be done a few lines 
lower when fc_req_to_bsgjob fails and blk_end_request() is called. Can you 
update the patch for this ?

-- james s


Sarang Radke wrote:
> A panic is seen when an ELS BSG request is send for rport which is online, but goes offline before the request is processed.
> 
> Here are the details of the issue and the fix.
> 
> . The request handler routine fc_bsg_handler checks if the rport is not online and tries to end the request by calling blk_end_request.
> . blk_end_request does not finish the request because it calls bidi_end_bidi_request with bidi_bytes = 0.
> . After 60 sec. the timeout handler, fc_bsg_job_timeout is invoked for this unfinished command.
> . A panic is seen in fc_bsg_job_timeout because req->special is NULL.
> . req->special  would have been set by function fc_req_to_bsgjob, if the rport was online.
> . The patch uses blk_end_request_all instead of blk_end_request which finishes the bidi request and timeout never happens.
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html