From: Omar Sandoval <osandov@osandov.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
Bart Van Assche <bart.vanassche@wdc.com>
Subject: Re: [PATCH] null_blk: fix zoned support for non-rq based operation
Date: Wed, 12 Sep 2018 17:04:55 -0700 [thread overview]
Message-ID: <20180913000455.GC6052@vader> (raw)
In-Reply-To: <5be22cec-2a80-9e07-6fe6-d5886295e848@kernel.dk>
On Wed, Sep 12, 2018 at 04:15:28PM -0600, Jens Axboe wrote:
> The supported added for zones in null_blk seem to assume that
> only rq based operation is possible. But this depends on the
> queue_mode setting, if this is set to 0, then cmd->bio is what
> we need to be operating on. Right now any attempt to load null_blk
> with queue_mode=0 will insta-crash, since cmd->rq is NULL and
> null_handle_cmd() assumes it to always be set.
>
> Make the zoned code deal with bio's instead, or pass in the
> appropriate sector/nr_sectors instead.
>
> Fixes: ca4b2a011948 ("null_blk: add zone support")
I just added block/023 to blktests for this, so
Tested-by: Omar Sandoval <osandov@fb.com>
> Signed-off-by: Jens Axboe <axboe@kernel.dk>
>
> ---
>
> diff --git a/drivers/block/null_blk.h b/drivers/block/null_blk.h
> index d81781f22dba..34e0030f0592 100644
> --- a/drivers/block/null_blk.h
> +++ b/drivers/block/null_blk.h
> @@ -87,10 +87,10 @@ struct nullb {
> #ifdef CONFIG_BLK_DEV_ZONED
> int null_zone_init(struct nullb_device *dev);
> void null_zone_exit(struct nullb_device *dev);
> -blk_status_t null_zone_report(struct nullb *nullb,
> - struct nullb_cmd *cmd);
> -void null_zone_write(struct nullb_cmd *cmd);
> -void null_zone_reset(struct nullb_cmd *cmd);
> +blk_status_t null_zone_report(struct nullb *nullb, struct bio *bio);
> +void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> + unsigned int nr_sectors);
> +void null_zone_reset(struct nullb_cmd *cmd, sector_t sector);
> #else
> static inline int null_zone_init(struct nullb_device *dev)
> {
> @@ -98,11 +98,14 @@ static inline int null_zone_init(struct nullb_device *dev)
> }
> static inline void null_zone_exit(struct nullb_device *dev) {}
> static inline blk_status_t null_zone_report(struct nullb *nullb,
> - struct nullb_cmd *cmd)
> + struct bio *bio)
> {
> return BLK_STS_NOTSUPP;
> }
> -static inline void null_zone_write(struct nullb_cmd *cmd) {}
> -static inline void null_zone_reset(struct nullb_cmd *cmd) {}
> +static inline void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> + unsigned int nr_sectors)
> +{
> +}
> +static inline void null_zone_reset(struct nullb_cmd *cmd, sector_t sector) {}
> #endif /* CONFIG_BLK_DEV_ZONED */
> #endif /* __NULL_BLK_H */
> diff --git a/drivers/block/null_blk_main.c b/drivers/block/null_blk_main.c
> index 6127e3ff7b4b..093b614d6524 100644
> --- a/drivers/block/null_blk_main.c
> +++ b/drivers/block/null_blk_main.c
> @@ -1157,16 +1157,33 @@ static void null_restart_queue_async(struct nullb *nullb)
> }
> }
>
> +static bool cmd_report_zone(struct nullb *nullb, struct nullb_cmd *cmd)
> +{
> + struct nullb_device *dev = cmd->nq->dev;
> +
> + if (dev->queue_mode == NULL_Q_BIO) {
> + if (bio_op(cmd->bio) == REQ_OP_ZONE_REPORT) {
> + cmd->error = null_zone_report(nullb, cmd->bio);
> + return true;
> + }
> + } else {
> + if (req_op(cmd->rq) == REQ_OP_ZONE_REPORT) {
> + cmd->error = null_zone_report(nullb, cmd->rq->bio);
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
> static blk_status_t null_handle_cmd(struct nullb_cmd *cmd)
> {
> struct nullb_device *dev = cmd->nq->dev;
> struct nullb *nullb = dev->nullb;
> int err = 0;
>
> - if (req_op(cmd->rq) == REQ_OP_ZONE_REPORT) {
> - cmd->error = null_zone_report(nullb, cmd);
> + if (cmd_report_zone(nullb, cmd))
> goto out;
> - }
>
> if (test_bit(NULLB_DEV_FL_THROTTLED, &dev->flags)) {
> struct request *rq = cmd->rq;
> @@ -1234,10 +1251,24 @@ static blk_status_t null_handle_cmd(struct nullb_cmd *cmd)
> cmd->error = errno_to_blk_status(err);
>
> if (!cmd->error && dev->zoned) {
> - if (req_op(cmd->rq) == REQ_OP_WRITE)
> - null_zone_write(cmd);
> - else if (req_op(cmd->rq) == REQ_OP_ZONE_RESET)
> - null_zone_reset(cmd);
> + sector_t sector;
> + unsigned int nr_sectors;
> + int op;
> +
> + if (dev->queue_mode == NULL_Q_BIO) {
> + op = bio_op(cmd->bio);
> + sector = cmd->bio->bi_iter.bi_sector;
> + nr_sectors = cmd->bio->bi_iter.bi_size >> 9;
> + } else {
> + op = req_op(cmd->rq);
> + sector = blk_rq_pos(cmd->rq);
> + nr_sectors = blk_rq_sectors(cmd->rq);
> + }
> +
> + if (op == REQ_OP_WRITE)
> + null_zone_write(cmd, sector, nr_sectors);
> + else if (op == REQ_OP_ZONE_RESET)
> + null_zone_reset(cmd, sector);
> }
> out:
> /* Complete IO by inline, softirq or timer */
> diff --git a/drivers/block/null_blk_zoned.c b/drivers/block/null_blk_zoned.c
> index a979ca00d7be..7c6b86d98700 100644
> --- a/drivers/block/null_blk_zoned.c
> +++ b/drivers/block/null_blk_zoned.c
> @@ -48,8 +48,8 @@ void null_zone_exit(struct nullb_device *dev)
> kvfree(dev->zones);
> }
>
> -static void null_zone_fill_rq(struct nullb_device *dev, struct request *rq,
> - unsigned int zno, unsigned int nr_zones)
> +static void null_zone_fill_bio(struct nullb_device *dev, struct bio *bio,
> + unsigned int zno, unsigned int nr_zones)
> {
> struct blk_zone_report_hdr *hdr = NULL;
> struct bio_vec bvec;
> @@ -57,7 +57,7 @@ static void null_zone_fill_rq(struct nullb_device *dev, struct request *rq,
> void *addr;
> unsigned int zones_to_cpy;
>
> - bio_for_each_segment(bvec, rq->bio, iter) {
> + bio_for_each_segment(bvec, bio, iter) {
> addr = kmap_atomic(bvec.bv_page);
>
> zones_to_cpy = bvec.bv_len / sizeof(struct blk_zone);
> @@ -84,29 +84,24 @@ static void null_zone_fill_rq(struct nullb_device *dev, struct request *rq,
> }
> }
>
> -blk_status_t null_zone_report(struct nullb *nullb,
> - struct nullb_cmd *cmd)
> +blk_status_t null_zone_report(struct nullb *nullb, struct bio *bio)
> {
> struct nullb_device *dev = nullb->dev;
> - struct request *rq = cmd->rq;
> - unsigned int zno = null_zone_no(dev, blk_rq_pos(rq));
> + unsigned int zno = null_zone_no(dev, bio->bi_iter.bi_sector);
> unsigned int nr_zones = dev->nr_zones - zno;
> - unsigned int max_zones = (blk_rq_bytes(rq) /
> - sizeof(struct blk_zone)) - 1;
> + unsigned int max_zones;
>
> + max_zones = (bio->bi_iter.bi_size / sizeof(struct blk_zone)) - 1;
> nr_zones = min_t(unsigned int, nr_zones, max_zones);
> -
> - null_zone_fill_rq(nullb->dev, rq, zno, nr_zones);
> + null_zone_fill_bio(nullb->dev, bio, zno, nr_zones);
>
> return BLK_STS_OK;
> }
>
> -void null_zone_write(struct nullb_cmd *cmd)
> +void null_zone_write(struct nullb_cmd *cmd, sector_t sector,
> + unsigned int nr_sectors)
> {
> struct nullb_device *dev = cmd->nq->dev;
> - struct request *rq = cmd->rq;
> - sector_t sector = blk_rq_pos(rq);
> - unsigned int rq_sectors = blk_rq_sectors(rq);
> unsigned int zno = null_zone_no(dev, sector);
> struct blk_zone *zone = &dev->zones[zno];
>
> @@ -118,7 +113,7 @@ void null_zone_write(struct nullb_cmd *cmd)
> case BLK_ZONE_COND_EMPTY:
> case BLK_ZONE_COND_IMP_OPEN:
> /* Writes must be at the write pointer position */
> - if (blk_rq_pos(rq) != zone->wp) {
> + if (sector != zone->wp) {
> cmd->error = BLK_STS_IOERR;
> break;
> }
> @@ -126,7 +121,7 @@ void null_zone_write(struct nullb_cmd *cmd)
> if (zone->cond == BLK_ZONE_COND_EMPTY)
> zone->cond = BLK_ZONE_COND_IMP_OPEN;
>
> - zone->wp += rq_sectors;
> + zone->wp += nr_sectors;
> if (zone->wp == zone->start + zone->len)
> zone->cond = BLK_ZONE_COND_FULL;
> break;
> @@ -137,11 +132,10 @@ void null_zone_write(struct nullb_cmd *cmd)
> }
> }
>
> -void null_zone_reset(struct nullb_cmd *cmd)
> +void null_zone_reset(struct nullb_cmd *cmd, sector_t sector)
> {
> struct nullb_device *dev = cmd->nq->dev;
> - struct request *rq = cmd->rq;
> - unsigned int zno = null_zone_no(dev, blk_rq_pos(rq));
> + unsigned int zno = null_zone_no(dev, sector);
> struct blk_zone *zone = &dev->zones[zno];
>
> zone->cond = BLK_ZONE_COND_EMPTY;
>
> --
> Jens Axboe
>
prev parent reply other threads:[~2018-09-13 0:04 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-09-12 22:15 [PATCH] null_blk: fix zoned support for non-rq based operation Jens Axboe
2018-09-13 0:04 ` Omar Sandoval [this message]
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=20180913000455.GC6052@vader \
--to=osandov@osandov.com \
--cc=axboe@kernel.dk \
--cc=bart.vanassche@wdc.com \
--cc=linux-block@vger.kernel.org \
/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).