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

[Boaz Harrosh <bharrosh@panasas.com>]

FUJITA Tomonori wrote:
> On Sun, 26 Oct 2008 11:38:04 +0200
> Boaz Harrosh <bharrosh@panasas.com> wrote:
> 
>> FUJITA Tomonori wrote:
>>> CC'ed Jens,
>>>
>>> On Wed, 22 Oct 2008 19:27:35 -0700
>>> Seokmann Ju <seokmann.ju@qlogic.com> wrote:
>>>
>>>> And it seems like that the panic is happening due to the fact that  
>>>> blk_delete_timer() is not called upon having completion of the service.
>>>> In other words, the block layer calls blk_add_timer() prior to  
>>>> dispatch the service but, it doesn't call blk_delete_timer() when it  
>>>> returned.
>>> Yeah, we need to call blk_delete_timer somewhere.
>>>
>>>
>>>> Just for heck of it, I've tried out by adding blk_delete_timer() in  
>>>> the ~/block/blk-exec.c:blk_end_sync_rq() and it seems fixes the problem.
>>> I think blk_end_sync_rq() is not the good place. From the perspective
>>> of bsg, we need to handle both blk_execute_rq_nowait and
>>> blk_execute_rq.
>>>
>>>
>>>> Seems like that there are APIs in the block layer that are call the  
>>>> blk_delete_timer(), including,
>>>> - blk_end_io()
>>>> - __blk_end_request()
>>>>
>>>> Could you guide me what is right way to fix the problem?
>>> Exporting blk_delete_timer is one option, but it doesn't look very
>>> nice (since the block layer doesn't export any details about its timer
>>> infrastructure), I think. Modifying blk_end_io() to make it usable for
>>> requests via something like bsg might be better.
>>>
>>> Anyway, we need to ask Jens.
>>>
>>> Jens, fc people have working on fc pass through support via bsg, which
>>> hooks bsg's request queue on fc transport objects (We did the similar
>>> thing for sas transport).
>>>
>>> We want the timeout feature for fc pass through and I think that it's
>>> nice to use the block layer timeout feature for it. But the users of
>>> bsg request queue don't need (or call) APIs such as
>>> end_that_request_last to call blk_delete_timer internally. How should
>>> these users call blk_delete_timer?
>> TOMO Hi
>> If a command is queued by bsg to a scsi device, which is posible. Then
>> blk_end_request() is called by scsi-ml. So it does work.
> 
> It doesn't work for bsg's scsi transport pass through stuff such as
> SMP (sas management protocol, we already support) and FC. Virtually,
> they don't use scsi-ml.
> 

Right, I know that, that's why I say.

> 
>> I think that all block-queue consumers should call one of
>> blk_end_request(),
> 
> This is kinda what I suggested in the previous mail but as I wrote,
> some of them don't now.
> 

I think they should, specially if they're going to use the timer.
The way I see it they must. It's kind of a block layer API thing.
Someone calls blk_execute_xx then eventually someone needs to call
blk_end_request. You could call it from bsg but only temporary until
all are fixed. (because you will need an ugly check to see if request
was not already ended)

> 
>> there are lots to choose from. We don't need
>> a new API. It will work with or without data, and it does what
>> you want.

Boaz