Re: [RFC] FC pass thru - Rev V

James Smart
2009-03-15T15:15:51Z

Boaz Harrosh wrote:
<snip..>
>> + if (rsp) {
>> + rsp_len = blk_rq_bytes(rsp);
>> + BUG_ON(job->reply->reply_payload_rcv_len > rsp_len);
>> + /* set reply (bidi) residual */
>> + rsp->data_len = (rsp_len - job->reply->reply_payload_rcv_len);
>> + }
>> +
>> + /* we assume all request payload was transferred */
>
> - /* we assume all request payload was transferred */
> + /* we assume all request payload was transferred, residual == 0 */
> + req->data_len = 0;
>
> Note: this is the proper procedure. Otherwise bsg user will receive transfer
> residual of all payload. On the other hand you might have more precise residual
> information here, as received from the scsi transport? (Specially in the error
> case, perhaps in the future).
>
>> + blk_end_request(req, err, blk_rq_bytes(req));
>
> - blk_end_request(req, err, blk_rq_bytes(req));
> + blk_end_bidi_request(req, err, req_len, rsp_len);
>
>> +
>> + fc_destroy_bsgjob(job);
>> +}
>
> This should take care of all possible cases, (bidi or otherwise), and will
> return the proper transfer residual count of 0 to user-mode.

We should be calling blk_end_bidi_request() even in cases where there wasn't a
req->next_rq ?

Here's what I would have done. Has your changes for setting req_len and
req->data_len, and Seokman's mods for calling either blk_end_bidi_request() or
blk_end_request() depending on whether there is a rsp request.

-- james s

/**
* fc_bsg_jobdone - completion routine for bsg requests that the LLD has
* completed
* @job: fc_bsg_job that is complete
*/
static void
fc_bsg_jobdone(struct fc_bsg_job *job)
{
struct request *req = job->req;
struct request *rsp = req->next_rq;
unsigned long flags;
unsigned rsp_len = 0, req_len = blk_rq_bytes(req);

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);
else
job->req->sense_len = job->reply_len;

/* we assume all request payload was transferred, residual == 0 */
req->data_len = 0;

if (rsp) {
rsp_len = blk_rq_bytes(rsp);
BUG_ON(job->reply->reply_payload_rcv_len > rsp_len);
/* set reply (bidi) residual */
rsp->data_len = (rsp_len - job->reply->reply_payload_rcv_len);
blk_end_bidi_request(req, err, req_len, rsp_len);
} else
blk_end_request(req, err, req_len);

fc_destroy_bsgjob(job);
}