From: "Javier González" <javier@javigon.com>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>,
Damien Le Moal <Damien.LeMoal@wdc.com>,
Hans Holmberg <hans@owltronix.com>,
linux-block@vger.kernel.org
Subject: Re: [PATCH 6/8] block: allocate the zone bitmaps lazily
Date: Tue, 3 Dec 2019 15:03:48 +0100 [thread overview]
Message-ID: <20191203140348.str5nt7oudu2prpn@mpHalley.local> (raw)
In-Reply-To: <20191203093908.24612-7-hch@lst.de>
On 03.12.2019 10:39, Christoph Hellwig wrote:
>Allocate the conventional zone bitmap and the sequential zone locking
>bitmap only when we find a zone of the respective type. This avoids
>wasting memory on the conventional zone bitmap for devices that only
>have sequential zones, and will also prepare for other future changes.
>
>Signed-off-by: Christoph Hellwig <hch@lst.de>
>---
> block/blk-zoned.c | 65 +++++++++++++++++++++++------------------------
> 1 file changed, 32 insertions(+), 33 deletions(-)
>
>diff --git a/block/blk-zoned.c b/block/blk-zoned.c
>index 9c3931051f4f..0131f9e14bd1 100644
>--- a/block/blk-zoned.c
>+++ b/block/blk-zoned.c
>@@ -342,6 +342,7 @@ struct blk_revalidate_zone_args {
> struct gendisk *disk;
> unsigned long *conv_zones_bitmap;
> unsigned long *seq_zones_wlock;
>+ unsigned int nr_zones;
> sector_t sector;
> };
>
>@@ -385,8 +386,22 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
> /* Check zone type */
> switch (zone->type) {
> case BLK_ZONE_TYPE_CONVENTIONAL:
>+ if (!args->conv_zones_bitmap) {
>+ args->conv_zones_bitmap =
>+ blk_alloc_zone_bitmap(q->node, args->nr_zones);
>+ if (!args->conv_zones_bitmap)
>+ return -ENOMEM;
>+ }
>+ set_bit(idx, args->conv_zones_bitmap);
>+ break;
> case BLK_ZONE_TYPE_SEQWRITE_REQ:
> case BLK_ZONE_TYPE_SEQWRITE_PREF:
>+ if (!args->seq_zones_wlock) {
>+ args->seq_zones_wlock =
>+ blk_alloc_zone_bitmap(q->node, args->nr_zones);
>+ if (!args->seq_zones_wlock)
>+ return -ENOMEM;
>+ }
> break;
> default:
> pr_warn("%s: Invalid zone type 0x%x at sectors %llu\n",
>@@ -394,37 +409,10 @@ static int blk_revalidate_zone_cb(struct blk_zone *zone, unsigned int idx,
> return -ENODEV;
> }
>
>- if (zone->type == BLK_ZONE_TYPE_CONVENTIONAL)
>- set_bit(idx, args->conv_zones_bitmap);
>-
> args->sector += zone->len;
> return 0;
> }
>
>-static int blk_update_zone_info(struct gendisk *disk, unsigned int nr_zones,
>- struct blk_revalidate_zone_args *args)
>-{
>- /*
>- * Ensure that all memory allocations in this context are done as
>- * if GFP_NOIO was specified.
>- */
>- unsigned int noio_flag = memalloc_noio_save();
>- struct request_queue *q = disk->queue;
>- int ret;
>-
>- args->seq_zones_wlock = blk_alloc_zone_bitmap(q->node, nr_zones);
>- if (!args->seq_zones_wlock)
>- return -ENOMEM;
>- args->conv_zones_bitmap = blk_alloc_zone_bitmap(q->node, nr_zones);
>- if (!args->conv_zones_bitmap)
>- return -ENOMEM;
>-
>- ret = disk->fops->report_zones(disk, 0, nr_zones,
>- blk_revalidate_zone_cb, args);
>- memalloc_noio_restore(noio_flag);
>- return ret;
>-}
>-
> /**
> * blk_revalidate_disk_zones - (re)allocate and initialize zone bitmaps
> * @disk: Target disk
>@@ -437,8 +425,10 @@ static int blk_update_zone_info(struct gendisk *disk, unsigned int nr_zones,
> int blk_revalidate_disk_zones(struct gendisk *disk)
> {
> struct request_queue *q = disk->queue;
>- unsigned int nr_zones = blkdev_nr_zones(disk);
>- struct blk_revalidate_zone_args args = { .disk = disk };
>+ struct blk_revalidate_zone_args args = {
>+ .disk = disk,
>+ .nr_zones = blkdev_nr_zones(disk),
>+ };
> int ret = 0;
>
> if (WARN_ON_ONCE(!blk_queue_is_zoned(q)))
>@@ -449,12 +439,21 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> * needs to be updated so that the sysfs exposed value is correct.
> */
> if (!queue_is_mq(q)) {
>- q->nr_zones = nr_zones;
>+ q->nr_zones = args.nr_zones;
> return 0;
> }
>
>- if (nr_zones)
>- ret = blk_update_zone_info(disk, nr_zones, &args);
>+ /*
>+ * Ensure that all memory allocations in this context are done as
>+ * if GFP_NOIO was specified.
>+ */
>+ if (args.nr_zones) {
>+ unsigned int noio_flag = memalloc_noio_save();
>+
>+ ret = disk->fops->report_zones(disk, 0, args.nr_zones,
>+ blk_revalidate_zone_cb, &args);
>+ memalloc_noio_restore(noio_flag);
>+ }
>
> /*
> * Install the new bitmaps, making sure the queue is stopped and
>@@ -463,7 +462,7 @@ int blk_revalidate_disk_zones(struct gendisk *disk)
> */
> blk_mq_freeze_queue(q);
> if (ret >= 0) {
>- q->nr_zones = nr_zones;
>+ q->nr_zones = args.nr_zones;
> swap(q->seq_zones_wlock, args.seq_zones_wlock);
> swap(q->conv_zones_bitmap, args.conv_zones_bitmap);
> ret = 0;
>--
>2.20.1
Looks good to me.
Reviewed-by: Javier González <javier@javigon.com>
next prev parent reply other threads:[~2019-12-03 14:03 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-03 9:39 avoid out of bounds zone bitmap access Christoph Hellwig
2019-12-03 9:39 ` [PATCH 1/8] null_blk: fix zone size paramter check Christoph Hellwig
2019-12-03 9:39 ` [PATCH 2/8] null_blk: cleanup null_gendisk_register Christoph Hellwig
2019-12-03 9:39 ` [PATCH 3/8] block: remove the empty line at the end of blk-zoned.c Christoph Hellwig
2019-12-03 9:39 ` [PATCH 4/8] block: simplify blkdev_nr_zones Christoph Hellwig
2019-12-03 9:39 ` [PATCH 5/8] block: replace seq_zones_bitmap with conv_zones_bitmap Christoph Hellwig
2019-12-03 14:02 ` Javier González
2019-12-03 9:39 ` [PATCH 6/8] block: allocate the zone bitmaps lazily Christoph Hellwig
2019-12-03 14:03 ` Javier González [this message]
2019-12-03 9:39 ` [PATCH 7/8] block: don't handle bio based drivers in blk_revalidate_disk_zones Christoph Hellwig
2019-12-03 14:04 ` Javier González
2019-12-03 9:39 ` [PATCH 8/8] block: set the zone size in blk_revalidate_disk_zones atomically Christoph Hellwig
2019-12-03 14:00 ` Javier González
2019-12-03 15:18 ` Christoph Hellwig
2019-12-03 15:34 ` Javier González
2019-12-03 15:42 ` Christoph Hellwig
2019-12-03 17:17 ` Javier González
2019-12-03 16:00 ` avoid out of bounds zone bitmap access Jens Axboe
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=20191203140348.str5nt7oudu2prpn@mpHalley.local \
--to=javier@javigon.com \
--cc=Damien.LeMoal@wdc.com \
--cc=axboe@kernel.dk \
--cc=hans@owltronix.com \
--cc=hch@lst.de \
--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