Re: [PATCH 1/2] scsi_transport_fc: FC pass through support via bsg interface - revised

Seokmann Ju
2008-10-28T16:03:53Z

On Oct 28, 2008, at 7:59 AM, Boaz Harrosh wrote:

> Boaz Harrosh wrote:
>> Boaz Harrosh wrote:
...
[snip]
...
>>> if the attached file is the code you tested then it is wrong look
>>> here:
>>>
>>>> +
>>>> + if (service->srv_reply.residual) {
>>>> + service->req->data_len = 0;
>>>> + service->req->next_rq->data_len = service->srv_reply.residual;
>>>> + } else {
>>>> + service->req->data_len = 0;
>>>> + service->req->next_rq->data_len = 0;
>>>> + }
>>>> +
>>> Move above to after the blk_end_bidi_request call
>>>
>>>> + blk_end_bidi_request(service->req, 0, blk_rq_bytes(service->req),
>>>> + service->req->next_rq ? blk_rq_bytes(service->req-
>>>> >next_rq) : 0);
>>> You must call blk_end_bidi_request before you change service->req-
>>> >data_len
>>> to hold the residual (or 0). Otherwise you damage the request.
>>>
>>>> + service->req->end_io(service->req, 0);
>>
>> Hmm, on re inspection req->end_io(...) called here has the same
>> problem.
>> Are you sure it's needed?
>>
>
> No you do not call req->end_io(..) directly. It eventual gets called
> by blk_end_bidi_request() inside end_that_request_last(). (Once
> all byte are completed)
Yes, that is correct. Thank you for the correction - end_io() call has
removed.
Other than setting req->errors field and req->next_rq->errors, there
are not
many things to do in the callback prior to calling
blk_end_bidi_request().

With the clean-up, updated patch will be followed.

Thank you,
Seokmann