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

[Boaz Harrosh <bharrosh@panasas.com>]

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.

I think that all block-queue consumers should call one of
blk_end_request(), 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.

Just my $0.017
Thanks Boaz