diff mbox series

null_blk: fix zoned support for non-rq based operation

Message ID 5be22cec-2a80-9e07-6fe6-d5886295e848@kernel.dk (mailing list archive)
State New, archived
Headers show
Series null_blk: fix zoned support for non-rq based operation | expand

Commit Message

Jens Axboe Sept. 12, 2018, 10:15 p.m. UTC 
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")
Signed-off-by: Jens Axboe <axboe@kernel.dk>

---

Comments

Omar Sandoval Sept. 13, 2018, 12:04 a.m. UTC | #1 
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
>
diff mbox series

Patch

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;