Re: [RFC PATCH 2/8] rqbased-dm: add block layer hook

business1
2006-12-21T22:56:13Z

Kiyoshi Ueda wrote:
>
> Hi Jens,
>
> On Thu, 21 Dec 2006 08:49:47 +0100, Jens Axboe <jens.axboe@oracle.com>
> wrote:
>> > The new hook is needed for error handling in dm.
>> > For example, when an error occurred on a request, dm-multipath
>> > wants to try another path before returning EIO to application.
>> > Without the new hook, at the point of end_that_request_last(),
>> > the bios are already finished with error and can't be retried.
>>
>> Ok, I see what you are getting at. The current ->end_io() is called when
>> the request has fully completed, you want notification for each chunk
>> potentially completed.
>>
>> I think a better design here would be to use ->end_io() as the full
>> completion handler, similar to how bio->bi_end_io() works. A request
>> originating from __make_request() would set something ala:
>>
>> int fs_end_io(struct request *rq, int error, unsigned int nr_bytes)
>> {
>> if (!__end_that_request_first(rq, err, nr_bytes)) {
>> end_that_request_last(rq, error);
>> return 0;
>> }
>>
>> return 1;
>> }
>>
>> and normal io completion from a driver would use a helper:
>>
>> int blk_complete_io(struct request *rq, int error, unsigned int nr_bytes)
>> {
>> return rq->end_io(rq, error, nr_bytes);
>> }
>>
>> instead of calling the functions manually. That would allow you to get
>> notification right at the beginning and do what you need, without adding
>> a special hook for this.
>
> I'm not confident about what you mean.
> Something like this?
> - __make_request() sets fs_end_io() to req->end_io()
> - The driver calls blk_complete_io()
> * if it succeeds, the request is done
> * if it fails, the request is not completed
> and the driver needs retry or something
> - Current users of req->end_io() have to update/rewrite thier end_io.
> - Features like mine will set its own end_io.
> It checks error and decides whether calling fs_end_io() or not.
>
> Depending on drivers, there are some functions called between
> __end_that_request_first() and end_that_request_last().
> For example:
> - add_disk_randomness()
> - blk_queue_end_tag()
> - floppy_off()
> So they might prevent such generalization.
>
>
> In addition to the suggested approach, what do you think about
> adding a new flag to req->cmd_flags which lets the end_io() handler
> not to return bio to upper layer?
> It will be useful for multipathing and can be done even within
> the current __end_that_request_first().
> For example,
>
> static int __end_that_request_first()
> {
> .....
> error = 0;
> if (end_io_error(uptodate))
> error = !uptodate ? -EIO : uptodate;
> .....
> if (error && (req->cmd_flags & "NEW_FLAG"))
> return 0; /* Tell the driver to call end_that_request_last() */
>
> total_types = bio_nbytes = 0;
> while ((bio = req->bio) != NULL) {
> ..... /* process of finishing bios */
> }
> .....
> }
>
> Thanks,
> Kiyoshi Ueda
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
>
> http://www.thebusinesssuccessgroup.com/Real-Estate-Investment-training.html

--
View this message in context: http://www.nabble.com/-RFC-PATCH-2-8--rqbased-dm%3A-add-block-layer-hook-tf2848786.html#a8016546
Sent from the linux-kernel mailing list archive at Nabble.com.