Re: Patch added to scsi-misc-2.6: [SCSI] libosd: Fix blk_put_request locking again

Re: Patch added to scsi-misc-2.6: [SCSI] libosd: Fix blk_put_request locking again

[Boaz Harrosh]

On 12/10/2009 05:49 PM, James Bottomley wrote:
> Your commit:
> 
>     [SCSI] libosd: Fix blk_put_request locking again
>     
>     So libosd has decided to sacrifice some code simplicity for the sake of
>     a clean API. One of these things is the possibility for users to call
>     osd_end_request, in any condition at any state. This opens up some
>     problems with calling blk_put_request when out-side of the completion
>     callback but calling __blk_put_request when detecting a from-completion
>     state.
>     
>     The current hack was working just fine until exofs decided to operate on
>     all devices in parallel and wait for the sum of the requests, before
>     deallocating all osd-requests at once. There are two new possible cases
>     1. All request in a group are deallocated as part of the last request's
>        async-done, request_queue is locked.
>     2. All request in a group where executed asynchronously, but
>        de-allocation was delayed to after the async-done, in the context of
>        another thread. Async execution but request_queue is not locked.
>     
>     The solution I chose was to separate the deallocation of the osd_request
>     which has the information users need, from the deallocation of the
>     internal(2) requests which impose the locking problem. The internal
>     block-requests are freed unconditionally inside the async-done-callback,
>     when we know the queue is always locked. If at osd_end_request time we
>     still have a bock-request, then we know it did not come from within an
>     async-done-callback and we can call the regular blk_put_request.
>     
>     The internal requests were used for carrying error information after
>     execution. This information is now copied to osd_request members for
>     later analysis by user code.
>     
>     The external API and behaviour was unchanged, except now it really
>     supports what was previously advertised.
>     
>     Reported-by: Vineet Agarwal <checkout.vineet@gmail.com>
>     Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
>     Cc: Stable Tree <stable@kernel.org>

The patch in it's current form will not apply to 2.6.32 and down. Since it is based
on current 2.6.33-rc0 code.

Anyway stable@kernel need *not* apply, because even though the bug does exist in older
Kernels, there where no clients that would exercise it, before 2.6.33-rc0.

>     Signed-off-by: James Bottomley <James.Bottomley@suse.de>
> 

Thanks
Boaz