From: Mike Snitzer <snitzer@redhat.com>
To: Jeffle Xu <jefflexu@linux.alibaba.com>
Cc: joseph.qi@linux.alibaba.com, dm-devel@redhat.com, koct9i@gmail.com
Subject: Re: [dm-devel] dm: add support for DM_TARGET_NOWAIT for various targets
Date: Wed, 11 Nov 2020 10:38:24 -0500 [thread overview]
Message-ID: <20201111153824.GB22834@redhat.com> (raw)
In-Reply-To: <20201110065558.22694-1-jefflexu@linux.alibaba.com>
On Tue, Nov 10 2020 at 1:55am -0500,
Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> This is one prep patch for supporting iopoll for dm device.
>
> The direct IO routine will set REQ_NOWAIT flag for REQ_HIPRI IO (that
> is, IO will do iopoll) in bio_set_polled(). Then in the IO submission
> routine, the ability of handling REQ_NOWAIT of the block device will
> be checked for REQ_HIPRI IO in submit_bio_checks(). -EOPNOTSUPP will
> be returned if the block device doesn't support REQ_NOWAIT.
submit_bio_checks() verifies the request_queue has QUEUE_FLAG_NOWAIT set
if the bio has REQ_NOWAIT.
> DM lacks support for REQ_NOWAIT until commit 6abc49468eea ("dm: add
> support for REQ_NOWAIT and enable it for linear target"). Since then,
> dm targets that support REQ_NOWAIT should advertise DM_TARGET_NOWAIT
> feature.
I'm not seeing why DM_TARGET_NOWAIT is needed (since you didn't add any
code that consumes the flag).
dm-table.c:dm_table_set_restrictions() has:
if (dm_table_supports_nowait(t))
blk_queue_flag_set(QUEUE_FLAG_NOWAIT, q);
else
blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, q);
> This patch adds support for DM_TARGET_NOWAIT for those dm targets, the
> .map() algorithm of which just involves sector recalculation.
So you're looking to constrain which targets will properly support
REQ_NOWAIT, based on whether they do a simple remapping?
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> Hi Mike,
>
> I could split these boilerplate code that each dm target have one
> seperate patch if you think that would be better.
One patch for all these is fine. But it should include the code that I
assume you'll be adding to dm_table_supports_nowait() to further verify
that the targets in the table are all DM_TARGET_NOWAIT.
And why isn't dm-linear setting DM_TARGET_NOWAIT?
Also, other targets _could_ be made to support REQ_NOWAIT by
conditionally returning bio_wouldblock_error() if appropriate
(e.g. bio-based dm-multipath's case of queue_if_no_path).
Mike
> ---
> drivers/md/dm-stripe.c | 2 +-
> drivers/md/dm-switch.c | 1 +
> drivers/md/dm-unstripe.c | 1 +
> drivers/md/dm-zero.c | 1 +
> 4 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-stripe.c b/drivers/md/dm-stripe.c
> index 151d022b032d..df359d33cda8 100644
> --- a/drivers/md/dm-stripe.c
> +++ b/drivers/md/dm-stripe.c
> @@ -496,7 +496,7 @@ static void stripe_io_hints(struct dm_target *ti,
> static struct target_type stripe_target = {
> .name = "striped",
> .version = {1, 6, 0},
> - .features = DM_TARGET_PASSES_INTEGRITY,
> + .features = DM_TARGET_PASSES_INTEGRITY | DM_TARGET_NOWAIT,
> .module = THIS_MODULE,
> .ctr = stripe_ctr,
> .dtr = stripe_dtr,
> diff --git a/drivers/md/dm-switch.c b/drivers/md/dm-switch.c
> index bff4c7fa1cd2..262e2b0fd975 100644
> --- a/drivers/md/dm-switch.c
> +++ b/drivers/md/dm-switch.c
> @@ -550,6 +550,7 @@ static int switch_iterate_devices(struct dm_target *ti,
> static struct target_type switch_target = {
> .name = "switch",
> .version = {1, 1, 0},
> + .features = DM_TARGET_NOWAIT,
> .module = THIS_MODULE,
> .ctr = switch_ctr,
> .dtr = switch_dtr,
> diff --git a/drivers/md/dm-unstripe.c b/drivers/md/dm-unstripe.c
> index e673dacf6418..7357c1bd5863 100644
> --- a/drivers/md/dm-unstripe.c
> +++ b/drivers/md/dm-unstripe.c
> @@ -178,6 +178,7 @@ static void unstripe_io_hints(struct dm_target *ti,
> static struct target_type unstripe_target = {
> .name = "unstriped",
> .version = {1, 1, 0},
> + .features = DM_TARGET_NOWAIT,
> .module = THIS_MODULE,
> .ctr = unstripe_ctr,
> .dtr = unstripe_dtr,
> diff --git a/drivers/md/dm-zero.c b/drivers/md/dm-zero.c
> index b65ca8dcfbdc..faa1dbffc8b4 100644
> --- a/drivers/md/dm-zero.c
> +++ b/drivers/md/dm-zero.c
> @@ -59,6 +59,7 @@ static int zero_map(struct dm_target *ti, struct bio *bio)
> static struct target_type zero_target = {
> .name = "zero",
> .version = {1, 1, 0},
> + .features = DM_TARGET_NOWAIT,
> .module = THIS_MODULE,
> .ctr = zero_ctr,
> .map = zero_map,
> --
> 2.27.0
>
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2020-11-11 15:39 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-10 6:55 [dm-devel] [PATCH] dm: add support for DM_TARGET_NOWAIT for various targets Jeffle Xu
2020-11-11 15:38 ` Mike Snitzer [this message]
2020-11-12 6:05 ` [dm-devel] " JeffleXu
2020-11-12 6:05 ` JeffleXu
2020-11-12 7:58 ` JeffleXu
2020-11-12 7:58 ` JeffleXu
2020-11-12 16:11 ` Mike Snitzer
2020-11-12 16:11 ` Mike Snitzer
2020-11-13 2:05 ` [dm-devel] [PATCH v2] " Jeffle Xu
2020-11-13 2:12 ` JeffleXu
2020-11-18 2:01 ` JeffleXu
2020-11-18 15:38 ` Mike Snitzer
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=20201111153824.GB22834@redhat.com \
--to=snitzer@redhat.com \
--cc=dm-devel@redhat.com \
--cc=jefflexu@linux.alibaba.com \
--cc=joseph.qi@linux.alibaba.com \
--cc=koct9i@gmail.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.