Re: [PATCH REPOST 1/2] rbd: encapsulate handling for a single request

[Alex Elder <elder@inktank.com>]

On 01/15/2013 05:45 PM, Josh Durgin wrote:
> On 01/03/2013 02:43 PM, Alex Elder wrote:
>> In rbd_rq_fn(), requests are fetched from the block layer and each
>> request is processed, looping through the request's list of bio's
>> until they've all been consumed.
>>
>> Separate the handling for a single request into its own function to
>> make it a bit easier to see what's going on.
>>
>> Signed-off-by: Alex Elder <elder@inktank.com>
>> ---

. . .

>> -
>> -        spin_lock_irq(q->queue_lock);
>> -
>> +        result = rbd_dev_do_request(rq, rbd_dev, snapc, ofs, size, bio);
>>           ceph_put_snap_context(snapc);
>> +        spin_lock_irq(q->queue_lock);
>> +        if (result < 0)
> 
> result may be 0 if num_segs == 0, which will make the request hang.
> I'm not sure if this will happen (or is even possible), but it's
> different from the previous behavior, which called
> __blk_end_request_all() in this case as well.

You're right.  And I think there may be some valid special
zero-length requests (like cache flush requests or something).

The value of num_segs comes from rbd_get_num_segments().  The
only way that will ever return 0 is if the len it is provided
is 0.

So my fix will be to change the check after the spin_lock_irq()
call from this:
	if (result < 0)
to this:
	if (!size || result < 0)

I'll re-post the result shortly.

					-Alex

>> +            __blk_end_request_all(rq, result);
>>       }
>>   }
>>
>