From: Ming Lei <ming.lei@redhat.com>
To: Damien Le Moal <damien.lemoal@wdc.com>,
dm-devel@redhat.com, Mike Snitzer <snitzer@redhat.com>
Cc: linux-block@vger.kernel.org, Jens Axboe <axboe@kernel.dk>,
linux-scsi@vger.kernel.org,
"Martin K . Petersen" <martin.petersen@oracle.com>
Subject: Re: [PATCH v4 5/7] block: Delay default elevator initialization
Date: Thu, 5 Sep 2019 15:19:24 +0800 [thread overview]
Message-ID: <20190905071923.GA8898@ming.t460p> (raw)
In-Reply-To: <20190905042901.5830-6-damien.lemoal@wdc.com>
On Thu, Sep 05, 2019 at 01:28:59PM +0900, Damien Le Moal wrote:
> When elevator_init_mq() is called from blk_mq_init_allocated_queue(),
> the only information known about the device is the number of hardware
> queues as the block device scan by the device driver is not completed
> yet. The device type and the device required features are not set yet,
> preventing to correctly choose the default elevator most suitable for
> the device.
>
> This currently affects all multi-queue zoned block devices which default
> to the "none" elevator instead of the required "mq-deadline" elevator.
> These drives currently include host-managed SMR disks connected to a
> smartpqi HBA and null_blk block devices with zoned mode enabled.
> Upcoming NVMe Zoned Namespace devices will also be affected.
>
> Fix this by moving the execution of elevator_init_mq() from
> blk_mq_init_allocated_queue() into __device_add_disk() to allow for the
> device driver to probe the device characteristics and set attributes
> of the device request queue prior to the elevator initialization. This
> initialization is skipped for DM devices using
> device_add_disk_no_queue_reg() as this also skips the queue
> registration.
>
> Additionally, to make sure that the elevator initialization is never
> done while requests are in-flight (there should be none when the device
> driver calls device_add_disk()), freeze and quiesce the device request
> queue before calling blk_mq_init_sched() in elevator_init_mq().
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> block/blk-mq.c | 2 --
> block/elevator.c | 7 +++++++
> block/genhd.c | 9 +++++++++
> 3 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index ee4caf0c0807..a37503984206 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -2902,8 +2902,6 @@ struct request_queue *blk_mq_init_allocated_queue(struct blk_mq_tag_set *set,
> blk_mq_add_queue_tag_set(set, q);
> blk_mq_map_swqueue(q);
>
> - elevator_init_mq(q);
> -
> return q;
>
> err_hctxs:
> diff --git a/block/elevator.c b/block/elevator.c
> index 520d6b224b74..096a670d22d7 100644
> --- a/block/elevator.c
> +++ b/block/elevator.c
> @@ -712,7 +712,14 @@ void elevator_init_mq(struct request_queue *q)
> if (!e)
> return;
>
> + blk_mq_freeze_queue(q);
> + blk_mq_quiesce_queue(q);
> +
> err = blk_mq_init_sched(q, e);
> +
> + blk_mq_unquiesce_queue(q);
> + blk_mq_unfreeze_queue(q);
> +
> if (err) {
> pr_warn("\"%s\" elevator initialization failed, "
> "falling back to \"none\"\n", e->elevator_name);
> diff --git a/block/genhd.c b/block/genhd.c
> index 54f1f0d381f4..26b31fcae217 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -695,6 +695,15 @@ static void __device_add_disk(struct device *parent, struct gendisk *disk,
> dev_t devt;
> int retval;
>
> + /*
> + * The disk queue should now be all set with enough information about
> + * the device for the elevator code to pick an adequate default
> + * elevator if one is needed, that is, for devices requesting queue
> + * registration.
> + */
> + if (register_queue)
> + elevator_init_mq(disk->queue);
> +
This way is better, but still changes the default elevator to 'none'
for dm-rq always.
thanks,
Ming
next prev parent reply other threads:[~2019-09-05 7:19 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 4:28 [PATCH v4 0/7] Elevator cleanups and improvements Damien Le Moal
2019-09-05 4:28 ` [PATCH v4 1/7] block: Cleanup elevator_init_mq() use Damien Le Moal
2019-09-05 4:28 ` [PATCH v4 2/7] block: Change elevator_init_mq() to always succeed Damien Le Moal
2019-09-05 4:28 ` [PATCH v4 3/7] block: Introduce elevator features Damien Le Moal
2019-09-05 4:28 ` [PATCH v4 4/7] block: Improve default elevator selection Damien Le Moal
2019-09-05 4:28 ` [PATCH v4 5/7] block: Delay default elevator initialization Damien Le Moal
2019-09-05 7:19 ` Ming Lei [this message]
2019-09-05 7:58 ` Damien Le Moal
2019-09-05 4:29 ` [PATCH v4 6/7] block: Set ELEVATOR_F_ZBD_SEQ_WRITE for nullblk zoned disks Damien Le Moal
2019-09-05 4:29 ` [PATCH v4 7/7] sd: Set ELEVATOR_F_ZBD_SEQ_WRITE for ZBC disks Damien Le Moal
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=20190905071923.GA8898@ming.t460p \
--to=ming.lei@redhat.com \
--cc=axboe@kernel.dk \
--cc=damien.lemoal@wdc.com \
--cc=dm-devel@redhat.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=snitzer@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).