From: Mike Snitzer <snitzer@redhat.com>
To: Jeffle Xu <jefflexu@linux.alibaba.com>
Cc: axboe@kernel.dk, xiaoguang.wang@linux.alibaba.com,
linux-block@vger.kernel.org, joseph.qi@linux.alibaba.com,
dm-devel@redhat.com, haoxu@linux.alibaba.com
Subject: Re: [dm-devel] [RFC 3/3] dm: add support for IO polling
Date: Wed, 21 Oct 2020 16:49:01 -0400 [thread overview]
Message-ID: <20201021204901.GB10896@redhat.com> (raw)
In-Reply-To: <20201020065420.124885-4-jefflexu@linux.alibaba.com>
On Tue, Oct 20 2020 at 2:54am -0400,
Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> Design of cookie is initially constrained as a per-bio concept. It
> dosn't work well when bio-split needed, and it is really an issue when
> adding support of iopoll for dm devices.
>
> The current algorithm implementation is simple. The returned cookie of
> dm device is actually not used since it is just the cookie of one of
> the cloned bios. Polling of dm device is actually polling on all
> hardware queues (in poll mode) of all underlying target devices.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> drivers/md/dm-core.h | 1 +
> drivers/md/dm-table.c | 30 ++++++++++++++++++++++++++++++
> drivers/md/dm.c | 39 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 70 insertions(+)
>
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index d522093cb39d..f18e066beffe 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -187,4 +187,5 @@ extern atomic_t dm_global_event_nr;
> extern wait_queue_head_t dm_global_eventq;
> void dm_issue_global_event(void);
>
> +int dm_io_poll(struct request_queue *q, blk_qc_t cookie);
> #endif
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index ce543b761be7..634b79842519 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1809,6 +1809,31 @@ static bool dm_table_requires_stable_pages(struct dm_table *t)
> return false;
> }
>
> +static int device_not_support_poll(struct dm_target *ti, struct dm_dev *dev,
> + sector_t start, sector_t len, void *data)
> +{
> + struct request_queue *q = bdev_get_queue(dev->bdev);
> +
> + return q && !(q->queue_flags & QUEUE_FLAG_POLL);
> +}
> +
> +bool dm_table_supports_poll(struct dm_table *t)
> +{
> + struct dm_target *ti;
> + unsigned int i;
> +
> + /* Ensure that all targets support DAX. */
> + for (i = 0; i < dm_table_get_num_targets(t); i++) {
> + ti = dm_table_get_target(t, i);
> +
> + if (!ti->type->iterate_devices ||
> + ti->type->iterate_devices(ti, device_not_support_poll, NULL))
> + return false;
> + }
> +
> + return true;
> +}
> +
> void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> struct queue_limits *limits)
> {
> @@ -1901,6 +1926,11 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> #endif
>
> blk_queue_update_readahead(q);
> +
> + if (dm_table_supports_poll(t)) {
> + q->poll_fn = dm_io_poll;
> + blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> + }
> }
You really want to implement the !dm_table_supports_poll case too on the
off chance that a DM table reload swaps in a new mapping that doesn't
support polling (be it the error target or whatever).
> unsigned int dm_table_get_num_targets(struct dm_table *t)
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index c18fc2548518..4eceaf87ffd4 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1666,6 +1666,45 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
> return ret;
> }
>
> +static int dm_poll_one_dev(struct request_queue *q, blk_qc_t cookie)
> +{
> + /* Iterate polling on all polling queues for mq device */
> + if (queue_is_mq(q)) {
> + struct blk_mq_hw_ctx *hctx;
> + int i, ret = 0;
> +
> + if (!percpu_ref_tryget(&q->q_usage_counter))
> + return 0;
> +
> + queue_for_each_poll_hw_ctx(q, hctx, i) {
> + ret += q->mq_ops->poll(hctx);
> + }
> +
> + percpu_ref_put(&q->q_usage_counter);
> + return ret;
> + } else
> + return q->poll_fn(q, cookie);
> +}
> +
> +int dm_io_poll(struct request_queue *q, blk_qc_t cookie)
> +{
> + struct mapped_device *md = q->queuedata;
> + struct dm_table *table;
> + struct dm_dev_internal *dd;
> + int srcu_idx;
> + int ret = 0;
> +
> + table = dm_get_live_table(md, &srcu_idx);
> + if (!table)
> + goto out;
> +
> + list_for_each_entry(dd, dm_table_get_devices(table), list)
> + ret += dm_poll_one_dev(bdev_get_queue(dd->dm_dev->bdev), cookie);
Definitely need to use ti->type->iterate_devices because otherwise you
could be iterating over non-data devices that have no business being
polled. Ideally you could use the cookie to find the unique bio in
question to quickly lookup the associated DM target to use
ti->type->iterate_devices
But short of having useful cookies, you should iterate through all
targets in the table and call ti->type->iterate_devices
Mike
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
WARNING: multiple messages have this Message-ID (diff)
From: Mike Snitzer <snitzer@redhat.com>
To: Jeffle Xu <jefflexu@linux.alibaba.com>
Cc: axboe@kernel.dk, linux-block@vger.kernel.org,
dm-devel@redhat.com, joseph.qi@linux.alibaba.com,
xiaoguang.wang@linux.alibaba.com, haoxu@linux.alibaba.com
Subject: Re: [RFC 3/3] dm: add support for IO polling
Date: Wed, 21 Oct 2020 16:49:01 -0400 [thread overview]
Message-ID: <20201021204901.GB10896@redhat.com> (raw)
In-Reply-To: <20201020065420.124885-4-jefflexu@linux.alibaba.com>
On Tue, Oct 20 2020 at 2:54am -0400,
Jeffle Xu <jefflexu@linux.alibaba.com> wrote:
> Design of cookie is initially constrained as a per-bio concept. It
> dosn't work well when bio-split needed, and it is really an issue when
> adding support of iopoll for dm devices.
>
> The current algorithm implementation is simple. The returned cookie of
> dm device is actually not used since it is just the cookie of one of
> the cloned bios. Polling of dm device is actually polling on all
> hardware queues (in poll mode) of all underlying target devices.
>
> Signed-off-by: Jeffle Xu <jefflexu@linux.alibaba.com>
> ---
> drivers/md/dm-core.h | 1 +
> drivers/md/dm-table.c | 30 ++++++++++++++++++++++++++++++
> drivers/md/dm.c | 39 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 70 insertions(+)
>
> diff --git a/drivers/md/dm-core.h b/drivers/md/dm-core.h
> index d522093cb39d..f18e066beffe 100644
> --- a/drivers/md/dm-core.h
> +++ b/drivers/md/dm-core.h
> @@ -187,4 +187,5 @@ extern atomic_t dm_global_event_nr;
> extern wait_queue_head_t dm_global_eventq;
> void dm_issue_global_event(void);
>
> +int dm_io_poll(struct request_queue *q, blk_qc_t cookie);
> #endif
> diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> index ce543b761be7..634b79842519 100644
> --- a/drivers/md/dm-table.c
> +++ b/drivers/md/dm-table.c
> @@ -1809,6 +1809,31 @@ static bool dm_table_requires_stable_pages(struct dm_table *t)
> return false;
> }
>
> +static int device_not_support_poll(struct dm_target *ti, struct dm_dev *dev,
> + sector_t start, sector_t len, void *data)
> +{
> + struct request_queue *q = bdev_get_queue(dev->bdev);
> +
> + return q && !(q->queue_flags & QUEUE_FLAG_POLL);
> +}
> +
> +bool dm_table_supports_poll(struct dm_table *t)
> +{
> + struct dm_target *ti;
> + unsigned int i;
> +
> + /* Ensure that all targets support DAX. */
> + for (i = 0; i < dm_table_get_num_targets(t); i++) {
> + ti = dm_table_get_target(t, i);
> +
> + if (!ti->type->iterate_devices ||
> + ti->type->iterate_devices(ti, device_not_support_poll, NULL))
> + return false;
> + }
> +
> + return true;
> +}
> +
> void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> struct queue_limits *limits)
> {
> @@ -1901,6 +1926,11 @@ void dm_table_set_restrictions(struct dm_table *t, struct request_queue *q,
> #endif
>
> blk_queue_update_readahead(q);
> +
> + if (dm_table_supports_poll(t)) {
> + q->poll_fn = dm_io_poll;
> + blk_queue_flag_set(QUEUE_FLAG_POLL, q);
> + }
> }
You really want to implement the !dm_table_supports_poll case too on the
off chance that a DM table reload swaps in a new mapping that doesn't
support polling (be it the error target or whatever).
> unsigned int dm_table_get_num_targets(struct dm_table *t)
> diff --git a/drivers/md/dm.c b/drivers/md/dm.c
> index c18fc2548518..4eceaf87ffd4 100644
> --- a/drivers/md/dm.c
> +++ b/drivers/md/dm.c
> @@ -1666,6 +1666,45 @@ static blk_qc_t dm_submit_bio(struct bio *bio)
> return ret;
> }
>
> +static int dm_poll_one_dev(struct request_queue *q, blk_qc_t cookie)
> +{
> + /* Iterate polling on all polling queues for mq device */
> + if (queue_is_mq(q)) {
> + struct blk_mq_hw_ctx *hctx;
> + int i, ret = 0;
> +
> + if (!percpu_ref_tryget(&q->q_usage_counter))
> + return 0;
> +
> + queue_for_each_poll_hw_ctx(q, hctx, i) {
> + ret += q->mq_ops->poll(hctx);
> + }
> +
> + percpu_ref_put(&q->q_usage_counter);
> + return ret;
> + } else
> + return q->poll_fn(q, cookie);
> +}
> +
> +int dm_io_poll(struct request_queue *q, blk_qc_t cookie)
> +{
> + struct mapped_device *md = q->queuedata;
> + struct dm_table *table;
> + struct dm_dev_internal *dd;
> + int srcu_idx;
> + int ret = 0;
> +
> + table = dm_get_live_table(md, &srcu_idx);
> + if (!table)
> + goto out;
> +
> + list_for_each_entry(dd, dm_table_get_devices(table), list)
> + ret += dm_poll_one_dev(bdev_get_queue(dd->dm_dev->bdev), cookie);
Definitely need to use ti->type->iterate_devices because otherwise you
could be iterating over non-data devices that have no business being
polled. Ideally you could use the cookie to find the unique bio in
question to quickly lookup the associated DM target to use
ti->type->iterate_devices
But short of having useful cookies, you should iterate through all
targets in the table and call ti->type->iterate_devices
Mike
next prev parent reply other threads:[~2020-10-21 20:49 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-20 6:54 [dm-devel] [RFC 0/3] Add support of iopoll for dm device Jeffle Xu
2020-10-20 6:54 ` Jeffle Xu
2020-10-20 6:54 ` [dm-devel] [RFC 1/3] block/mq: add iterator for polling hw queues Jeffle Xu
2020-10-20 6:54 ` Jeffle Xu
2020-10-20 6:54 ` [dm-devel] [RFC 2/3] block: add back ->poll_fn in request queue Jeffle Xu
2020-10-20 6:54 ` Jeffle Xu
2020-10-20 6:54 ` [dm-devel] [RFC 3/3] dm: add support for IO polling Jeffle Xu
2020-10-20 6:54 ` Jeffle Xu
2020-10-21 20:49 ` Mike Snitzer [this message]
2020-10-21 20:49 ` Mike Snitzer
2020-10-22 3:14 ` [dm-devel] " Ming Lei
2020-10-22 3:14 ` Ming Lei
2020-10-21 20:39 ` [dm-devel] [RFC 0/3] Add support of iopoll for dm device Mike Snitzer
2020-10-21 20:39 ` Mike Snitzer
2020-10-22 5:28 ` [dm-devel] " JeffleXu
2020-10-22 5:28 ` JeffleXu
2020-10-26 18:53 ` [dm-devel] " Mike Snitzer
2020-10-26 18:53 ` Mike Snitzer
2020-11-02 3:14 ` [dm-devel] " JeffleXu
2020-11-02 3:14 ` JeffleXu
2020-11-02 15:28 ` [dm-devel] " Mike Snitzer
2020-11-02 15:28 ` Mike Snitzer
2020-11-03 8:59 ` [dm-devel] " JeffleXu
2020-11-03 8:59 ` JeffleXu
2020-11-04 6:47 ` [dm-devel] " JeffleXu
2020-11-04 6:47 ` JeffleXu
2020-11-04 15:08 ` [dm-devel] " Mike Snitzer
2020-11-04 15:08 ` Mike Snitzer
2020-11-06 2:51 ` [dm-devel] " JeffleXu
2020-11-06 2:51 ` JeffleXu
2020-11-06 2:57 ` JeffleXu
2020-11-06 17:45 ` Mike Snitzer
2020-11-06 17:45 ` Mike Snitzer
2020-11-08 1:09 ` [dm-devel] " JeffleXu
2020-11-08 1:09 ` JeffleXu
2020-11-09 18:15 ` Mike Snitzer
2020-11-09 18:15 ` Mike Snitzer
2020-11-10 1:43 ` [dm-devel] " JeffleXu
2020-11-10 1:43 ` JeffleXu
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=20201021204901.GB10896@redhat.com \
--to=snitzer@redhat.com \
--cc=axboe@kernel.dk \
--cc=dm-devel@redhat.com \
--cc=haoxu@linux.alibaba.com \
--cc=jefflexu@linux.alibaba.com \
--cc=joseph.qi@linux.alibaba.com \
--cc=linux-block@vger.kernel.org \
--cc=xiaoguang.wang@linux.alibaba.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.