Re: [PATCH 4/9] block: Define zoned block device operations

[Bart Van Assche]

On 09/19/16 14:27, Damien Le Moal wrote:

diff --git a/block/blk-core.c b/block/blk-core.c
index 36c7ac3..4a7f7ba 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1941,6 +1941,13 @@ generic_make_request_checks(struct bio *bio)
case REQ_OP_WRITE_SAME:
if (!bdev_write_same(bio->bi_bdev))
goto not_supported;
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
+   if (!bdev_zoned(bio->bi_bdev))
+   goto not_supported;


In patch 1/9 the BLK_ZONED_* constants have been introduced. The above 
code compares the bdev_zoned() return value against 0. That means that 
the above code will break if the numeric value of the BLK_ZONED_* 
constants would be changed. Please change the above code such that it 
compares the bdev_zoned() return value against a BLK_ZONED_* constant.



+ * Operation:
+ * REQ_OP_ZONE_REPORT: Request information for all zones or for a single zone.
+ * REQ_OP_ZONE_RESET: Reset the write pointer of all zones or of a single zone.
+ * REQ_OP_ZONE_OPEN: Explicitely open the maximum allowed number of zones or
+ *   a single zone. For the former case, the zones that will
+ *   actually be open are chosen by the disk.
+ * REQ_OP_ZONE_CLOSE: Close all implicitely or explicitely open zones or
+ *a single zone.
+ * REQ_OP_ZONE_FINISH: Transition one or all open and closed zones to the full
+ * condition.


Please change *plicitely into *plicitly.

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/9] block: Define zoned block device operations

[Damien Le Moal]

From: Shaun Tancheff 

Define REQ_OP_ZONE_REPORT, REQ_OP_ZONE_RESET, REQ_OP_ZONE_OPEN,
REQ_OP_ZONE_CLOSE and REQ_OP_ZONE_FINISH for handling zones of
zoned block devices (host-managed and host-aware). With with these
new commands, the total number of operations defined reaches 11 and
requires increasing REQ_OP_BITS from 3 to 4.

Signed-off-by: Shaun Tancheff 

Changelog (Damien):
All requests have no payload and may operate on all zones of the
device (when the BIO sector and size are 0) or on a single zone
(when the BIO sector and size are aigned on a zone).

REQ_OP_ZONE_REPORT is not sent directly to the device
and is processed in sd_zbc.c using the device zone work
in order to parse the report reply and manage changes to
the zone information cache of the device.

Signed-off-by: Damien Le Moal 
---
 block/blk-core.c  |  7 +++
 block/blk-merge.c | 31 +++
 include/linux/bio.h   | 36 +++-
 include/linux/blk_types.h | 27 ++-
 4 files changed, 87 insertions(+), 14 deletions(-)

diff --git a/block/blk-core.c b/block/blk-core.c
index 36c7ac3..4a7f7ba 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1941,6 +1941,13 @@ generic_make_request_checks(struct bio *bio)
case REQ_OP_WRITE_SAME:
if (!bdev_write_same(bio->bi_bdev))
goto not_supported;
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
+   if (!bdev_zoned(bio->bi_bdev))
+   goto not_supported;
break;
default:
break;
diff --git a/block/blk-merge.c b/block/blk-merge.c
index 2642e5f..f9299df 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -202,6 +202,21 @@ void blk_queue_split(struct request_queue *q, struct bio 
**bio,
case REQ_OP_WRITE_SAME:
split = blk_bio_write_same_split(q, *bio, bs, &nsegs);
break;
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
+   /*
+* For these commands, bi_size is either 0 to specify
+* operation on the entire block device sector range,
+* or a zone size for operation on a single zone.
+* Since a zone size may be much bigger than the maximum
+* allowed BIO size, we cannot use blk_bio_segment_split.
+*/
+   split = NULL;
+   nsegs = 0;
+   break;
default:
split = blk_bio_segment_split(q, *bio, q->bio_split, &nsegs);
break;
@@ -241,11 +256,19 @@ static unsigned int __blk_recalc_rq_segments(struct 
request_queue *q,
 * This should probably be returning 0, but blk_add_request_payload()
 * (Christoph)
 */
-   if (bio_op(bio) == REQ_OP_DISCARD || bio_op(bio) == REQ_OP_SECURE_ERASE)
-   return 1;
-
-   if (bio_op(bio) == REQ_OP_WRITE_SAME)
+   switch(bio_op(bio)) {
+   case REQ_OP_DISCARD:
+   case REQ_OP_SECURE_ERASE:
+   case REQ_OP_WRITE_SAME:
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
return 1;
+   default:
+   break;
+   }
 
fbio = bio;
cluster = blk_queue_cluster(q);
diff --git a/include/linux/bio.h b/include/linux/bio.h
index 23ddf4b..d9c2e21 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -69,20 +69,38 @@
  */
 static inline bool bio_has_data(struct bio *bio)
 {
-   if (bio &&
-   bio->bi_iter.bi_size &&
-   bio_op(bio) != REQ_OP_DISCARD &&
-   bio_op(bio) != REQ_OP_SECURE_ERASE)
-   return true;
+   if (!bio || !bio->bi_iter.bi_size)
+   return false;
 
-   return false;
+   switch (bio_op(bio)) {
+   case REQ_OP_DISCARD:
+   case REQ_OP_SECURE_ERASE:
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
+   return false;
+   default:
+   return true;
+   }
 }
 
 static inline bool bio_no_advance_iter(struct bio *bio)
 {
-   return bio_op(bio) == REQ_OP_DISCARD ||
-  bio_op(bio) == REQ_OP_SECURE_ERASE ||
-  bio_op(bio) == REQ_OP_WRITE_SAME;
+   switch (bio_op(bio)) {
+   case REQ_OP_DISCARD:
+   case REQ_OP_SECURE_ERASE:
+   case REQ_OP_WRITE_SAME:
+   case REQ_OP_ZONE_REPORT:
+   case REQ_OP_ZONE_RESET:
+   case REQ_OP_ZONE_OPEN:
+   case REQ_OP_ZONE_CLOSE:
+   case REQ_OP_ZONE_FINISH:
+   return true;
+   default:
+