On Wed, 17 Jun 2009 09:31:30 -0700
Giridhar Malavali <giridhar.malavali@xxxxxxxxxx> wrote:
>
> Registered the softirq_done function for BSG request since this is
> requried by the block layer using block level
> request timeout functionality. This function will be called by the
> block layer as part of time out clean process to release the BSG
> request.
>
> Moved some of the BSG request completion activities to softirq_done
> routine to take care of both normal and timout completions.
>
> Signed-off-by: Giridhar Malavali <giridhar.malavali@xxxxxxxxxx>
The patch seems to be malformed to me.
> drivers/scsi/scsi_transport_fc.c | 37 ++++++++++++++++++
> +------------------
> 1 files changed, 19 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/scsi/scsi_transport_fc.c b/drivers/scsi/
> scsi_transport_fc.c
> index 3f64d93..453d9e6 100644
> --- a/drivers/scsi/scsi_transport_fc.c
> +++ b/drivers/scsi/scsi_transport_fc.c
> @@ -3397,7 +3397,6 @@ fc_destroy_bsgjob(struct fc_bsg_job *job)
> kfree(job);
> }
>
> -
> /**
> * fc_bsg_jobdone - completion routine for bsg requests that the LLD
> has
> * completed
> @@ -3408,15 +3407,10 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
> {
> struct request *req = job->req;
> struct request *rsp = req->next_rq;
> - unsigned long flags;
> int err;
>
> - spin_lock_irqsave(&job->job_lock, flags);
> - job->state_flags |= FC_RQST_STATE_DONE;
> - job->ref_cnt--;
> - spin_unlock_irqrestore(&job->job_lock, flags);
> -
> err = job->req->errors = job->reply->result;
> +
> if (err < 0)
> /* we're only returning the result field in the reply */
> job->req->sense_len = sizeof(uint32_t);
> @@ -3433,13 +3427,27 @@ fc_bsg_jobdone(struct fc_bsg_job *job)
> rsp->resid_len -= min(job->reply->reply_payload_rcv_len,
> rsp->resid_len);
> }
> + blk_complete_request(req);
> +}
> +
> +/**
> + * fc_bsg_softirq_done - softirq done routine for destroying the bsg
> requests
> + * @req: BSG request that holds the job to be destroyed
> + */
> +static void fc_bsg_softirq_done(struct request *rq)
> +{
> + struct fc_bsg_job *job = rq->special;
> + unsigned long flags;
>
> - blk_end_request_all(req, err);
> + spin_lock_irqsave(&job->job_lock, flags);
> + job->state_flags |= FC_RQST_STATE_DONE;
> + job->ref_cnt--;
> + spin_unlock_irqrestore(&job->job_lock, flags);
>
> + blk_end_request_all(rq, rq->errors);
> fc_destroy_bsgjob(job);
> }
>
> -
> /**
> * fc_bsg_job_timeout - handler for when a bsg request timesout
> * @req: request that timed out
> @@ -3471,19 +3479,10 @@ fc_bsg_job_timeout(struct request *req)
> "abort failed with status %d\n", err);
> }
>
> - if (!done) {
> - spin_lock_irqsave(&job->job_lock, flags);
> - job->ref_cnt--;
> - spin_unlock_irqrestore(&job->job_lock, flags);
> - fc_destroy_bsgjob(job);
> - }
> -
> /* the blk_end_sync_io() doesn't check the error */
> return BLK_EH_HANDLED;
I think that you need to return a proper value here; see my previous
patch. If a job is done, we need to return BLK_EH_NOT_HANDLED here;
otherwise the block layer calls __blk_complete_request for the job
(and later we call blk_complete_request for the same job).
The rest looks fine by me.
--
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
