From: Jens Axboe <jens.axboe@oracle.com>
To: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Cc: dm-devel@redhat.com, mchristi@redhat.com,
linux-kernel@vger.kernel.org, agk@redhat.com
Subject: Re: [RFC PATCH 2/8] rqbased-dm: add block layer hook
Date: Thu, 21 Dec 2006 08:49:47 +0100 [thread overview]
Message-ID: <20061221074947.GC17199@kernel.dk> (raw)
In-Reply-To: <20061220.165246.85417944.k-ueda@ct.jp.nec.com>
On Wed, Dec 20 2006, Kiyoshi Ueda wrote:
> Hi Jens,
>
> Sorry for the less explanation.
>
> On Wed, 20 Dec 2006 14:49:24 +0100, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Tue, Dec 19 2006, Kiyoshi Ueda wrote:
> > > This patch adds new "end_io_first" hook in __end_that_request_first()
> > > for request-based device-mapper.
> >
> > What's this for, lack of stacking?
>
> I don't understand the meaning of "lack of stacking" well but
> I guess that it means "Is the existing hook in end_that_request_last()
> not enough?" If so, the answer is no.
> (If the geuss is wrong, please let me know.)
>
> 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.
When designing these things, never be afraid to change some of the core
bits. It is a lot better than hacking around the current code, if it
doesn't quite fit your needs.
--
Jens Axboe
WARNING: multiple messages have this Message-ID (diff)
From: Jens Axboe <jens.axboe@oracle.com>
To: Kiyoshi Ueda <k-ueda@ct.jp.nec.com>
Cc: agk@redhat.com, mchristi@redhat.com,
linux-kernel@vger.kernel.org, dm-devel@redhat.com,
j-nomura@ce.jp.nec.com
Subject: Re: [RFC PATCH 2/8] rqbased-dm: add block layer hook
Date: Thu, 21 Dec 2006 08:49:47 +0100 [thread overview]
Message-ID: <20061221074947.GC17199@kernel.dk> (raw)
In-Reply-To: <20061220.165246.85417944.k-ueda@ct.jp.nec.com>
On Wed, Dec 20 2006, Kiyoshi Ueda wrote:
> Hi Jens,
>
> Sorry for the less explanation.
>
> On Wed, 20 Dec 2006 14:49:24 +0100, Jens Axboe <jens.axboe@oracle.com> wrote:
> > On Tue, Dec 19 2006, Kiyoshi Ueda wrote:
> > > This patch adds new "end_io_first" hook in __end_that_request_first()
> > > for request-based device-mapper.
> >
> > What's this for, lack of stacking?
>
> I don't understand the meaning of "lack of stacking" well but
> I guess that it means "Is the existing hook in end_that_request_last()
> not enough?" If so, the answer is no.
> (If the geuss is wrong, please let me know.)
>
> 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.
When designing these things, never be afraid to change some of the core
bits. It is a lot better than hacking around the current code, if it
doesn't quite fit your needs.
--
Jens Axboe
next prev parent reply other threads:[~2006-12-21 7:49 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-12-19 22:11 [RFC PATCH 2/8] rqbased-dm: add block layer hook Kiyoshi Ueda
2006-12-19 22:11 ` Kiyoshi Ueda
2006-12-20 13:49 ` Jens Axboe
2006-12-20 13:49 ` Jens Axboe
2006-12-20 21:52 ` Kiyoshi Ueda
2006-12-20 21:52 ` Kiyoshi Ueda
2006-12-21 7:49 ` Jens Axboe [this message]
2006-12-21 7:49 ` Jens Axboe
2006-12-21 22:22 ` Kiyoshi Ueda
2006-12-21 22:22 ` Kiyoshi Ueda
2006-12-21 22:56 ` business1
2006-12-22 7:18 ` Mike Christie
2006-12-22 7:18 ` [dm-devel] " Mike Christie
2006-12-22 19:55 ` Kiyoshi Ueda
2006-12-22 19:55 ` [dm-devel] " Kiyoshi Ueda
2006-12-21 23:00 ` business1
2006-12-21 22:56 ` business1
2006-12-21 22:53 ` business1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20061221074947.GC17199@kernel.dk \
--to=jens.axboe@oracle.com \
--cc=agk@redhat.com \
--cc=dm-devel@redhat.com \
--cc=k-ueda@ct.jp.nec.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mchristi@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.