diff mbox series

[v4,7/8] block: introduce blk_queue_clear_zone_settings()

Message ID 20210128044733.503606-8-damien.lemoal@wdc.com (mailing list archive)
State New, archived
Headers show
Series block: add zone write granularity limit | expand

Commit Message

Damien Le Moal Jan. 28, 2021, 4:47 a.m. UTC
Introduce the internal function blk_queue_clear_zone_settings() to
cleanup all limits and resources related to zoned block devices. This
new function is called from blk_queue_set_zoned() when a disk zoned
model is set to BLK_ZONED_NONE. This particular case can happens when a
partition is created on a host-aware scsi disk.

Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
---
 block/blk-settings.c |  2 ++
 block/blk-zoned.c    | 17 +++++++++++++++++
 block/blk.h          |  2 ++
 3 files changed, 21 insertions(+)

Comments

Chaitanya Kulkarni Jan. 28, 2021, 5:26 a.m. UTC | #1
On 1/27/21 20:47, Damien Le Moal wrote:
> Introduce the internal function blk_queue_clear_zone_settings() to
> cleanup all limits and resources related to zoned block devices. This
> new function is called from blk_queue_set_zoned() when a disk zoned
> model is set to BLK_ZONED_NONE. This particular case can happens when a
> partition is created on a host-aware scsi disk.
>
> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com>
nit:- can be done at the time of applying the patch.
In the last line of the commit log :-

1. s/happens/happen/.
2. article 'a' on the same line with the noun 'partition'.

see it make sense otherwise ignore :-

Introduce the internal function blk_queue_clear_zone_settings() to
cleanup all limits and resources related to zoned block devices. This
new function is called from blk_queue_set_zoned() when a disk zoned
model is set to BLK_ZONED_NONE. This particular case can happen when
apartition is created on a host-aware scsi disk.

Either case LGTM,

Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
Christoph Hellwig Jan. 28, 2021, 9:21 a.m. UTC | #2
On Thu, Jan 28, 2021 at 01:47:32PM +0900, Damien Le Moal wrote:
> Introduce the internal function blk_queue_clear_zone_settings() to
> cleanup all limits and resources related to zoned block devices. This
> new function is called from blk_queue_set_zoned() when a disk zoned
> model is set to BLK_ZONED_NONE. This particular case can happens when a
> partition is created on a host-aware scsi disk.

Shouldn't we just do all this work when blk_queue_set_zoned is called
with a BLK_ZONED_NONE argument?  That seems like the more obvious API
to me.
Damien Le Moal Jan. 28, 2021, 9:32 a.m. UTC | #3
On 2021/01/28 18:21, Christoph Hellwig wrote:
> On Thu, Jan 28, 2021 at 01:47:32PM +0900, Damien Le Moal wrote:
>> Introduce the internal function blk_queue_clear_zone_settings() to
>> cleanup all limits and resources related to zoned block devices. This
>> new function is called from blk_queue_set_zoned() when a disk zoned
>> model is set to BLK_ZONED_NONE. This particular case can happens when a
>> partition is created on a host-aware scsi disk.
> 
> Shouldn't we just do all this work when blk_queue_set_zoned is called
> with a BLK_ZONED_NONE argument?  That seems like the more obvious API
> to me.

That is what I did. blk_queue_set_zoned() calls blk_queue_clear_zone_settings()
for BLK_ZONED_NONE case. I simply did not open code the cleanups in that
functions because it is simpler to stub only blk_queue_clear_zone_settings()
rather than having conditionals in blk_queue_set_zoned(). That also puts the
cleanup function together with the code that allocates most resources in
blk-zoned.c. Easier to not overlook something.
Christoph Hellwig Jan. 28, 2021, 9:33 a.m. UTC | #4
On Thu, Jan 28, 2021 at 09:32:22AM +0000, Damien Le Moal wrote:
> On 2021/01/28 18:21, Christoph Hellwig wrote:
> > On Thu, Jan 28, 2021 at 01:47:32PM +0900, Damien Le Moal wrote:
> >> Introduce the internal function blk_queue_clear_zone_settings() to
> >> cleanup all limits and resources related to zoned block devices. This
> >> new function is called from blk_queue_set_zoned() when a disk zoned
> >> model is set to BLK_ZONED_NONE. This particular case can happens when a
> >> partition is created on a host-aware scsi disk.
> > 
> > Shouldn't we just do all this work when blk_queue_set_zoned is called
> > with a BLK_ZONED_NONE argument?  That seems like the more obvious API
> > to me.
> 
> That is what I did. blk_queue_set_zoned() calls blk_queue_clear_zone_settings()
> for BLK_ZONED_NONE case. I simply did not open code the cleanups in that
> functions because it is simpler to stub only blk_queue_clear_zone_settings()
> rather than having conditionals in blk_queue_set_zoned(). That also puts the
> cleanup function together with the code that allocates most resources in
> blk-zoned.c. Easier to not overlook something.

Ok, looks good to me then:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Johannes Thumshirn Jan. 28, 2021, 11:43 a.m. UTC | #5
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
diff mbox series

Patch

diff --git a/block/blk-settings.c b/block/blk-settings.c
index a1e66165adcf..7dd8be314ac6 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -910,6 +910,8 @@  void blk_queue_set_zoned(struct gendisk *disk, enum blk_zoned_model model)
 		 */
 		blk_queue_zone_write_granularity(q,
 						queue_logical_block_size(q));
+	} else {
+		blk_queue_clear_zone_settings(q);
 	}
 }
 EXPORT_SYMBOL_GPL(blk_queue_set_zoned);
diff --git a/block/blk-zoned.c b/block/blk-zoned.c
index 7a68b6e4300c..833978c02e60 100644
--- a/block/blk-zoned.c
+++ b/block/blk-zoned.c
@@ -549,3 +549,20 @@  int blk_revalidate_disk_zones(struct gendisk *disk,
 	return ret;
 }
 EXPORT_SYMBOL_GPL(blk_revalidate_disk_zones);
+
+void blk_queue_clear_zone_settings(struct request_queue *q)
+{
+	blk_mq_freeze_queue(q);
+
+	blk_queue_free_zone_bitmaps(q);
+	blk_queue_flag_clear(QUEUE_FLAG_ZONE_RESETALL, q);
+	q->required_elevator_features &= ~ELEVATOR_F_ZBD_SEQ_WRITE;
+	q->nr_zones = 0;
+	q->max_open_zones = 0;
+	q->max_active_zones = 0;
+	q->limits.chunk_sectors = 0;
+	q->limits.zone_write_granularity = 0;
+	q->limits.max_zone_append_sectors = 0;
+
+	blk_mq_unfreeze_queue(q);
+}
diff --git a/block/blk.h b/block/blk.h
index 0198335c5838..977d79a0d99a 100644
--- a/block/blk.h
+++ b/block/blk.h
@@ -333,8 +333,10 @@  struct bio *blk_next_bio(struct bio *bio, unsigned int nr_pages, gfp_t gfp);
 
 #ifdef CONFIG_BLK_DEV_ZONED
 void blk_queue_free_zone_bitmaps(struct request_queue *q);
+void blk_queue_clear_zone_settings(struct request_queue *q);
 #else
 static inline void blk_queue_free_zone_bitmaps(struct request_queue *q) {}
+static inline void blk_queue_clear_zone_settings(struct request_queue *q) {}
 #endif
 
 int blk_alloc_devt(struct block_device *part, dev_t *devt);