From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: Hannes Reinecke <hare@suse.de>, Mike Snitzer <snitzer@redhat.com>
Cc: "dm-devel@redhat.com" <dm-devel@redhat.com>
Subject: Re: [PATCH 06/15] dm-zoned: temporary superblock for tertiary devices
Date: Thu, 28 May 2020 02:54:17 +0000 [thread overview]
Message-ID: <CY4PR04MB37510485B74E55A17B4106BDE78E0@CY4PR04MB3751.namprd04.prod.outlook.com> (raw)
In-Reply-To: 20200527062225.72849-7-hare@suse.de
On 2020/05/27 15:22, Hannes Reinecke wrote:
> Checking the tertiary superblock just consists of validating UUIDs,
> crcs, and the generation number; it doesn't have contents which
> would be required during the actual operation.
> So we should be allocating a temporary superblock when checking
> tertiary devices and avoid having to store it together with the
> 'real' superblocks.
>
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> ---
> drivers/md/dm-zoned-metadata.c | 104 ++++++++++++++++++++++-------------------
> 1 file changed, 57 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
> index 839f9078806d..bb9ce72bf18c 100644
> --- a/drivers/md/dm-zoned-metadata.c
> +++ b/drivers/md/dm-zoned-metadata.c
> @@ -174,7 +174,7 @@ struct dmz_metadata {
> /* Zone information array */
> struct xarray zones;
>
> - struct dmz_sb sb[3];
> + struct dmz_sb sb[2];
> unsigned int mblk_primary;
> unsigned int sb_version;
> u64 sb_gen;
> @@ -1014,10 +1014,11 @@ int dmz_flush_metadata(struct dmz_metadata *zmd)
> /*
> * Check super block.
> */
> -static int dmz_check_sb(struct dmz_metadata *zmd, unsigned int set)
> +static int dmz_check_sb(struct dmz_metadata *zmd, struct dmz_sb *dsb,
> + bool tertiary)
> {
> - struct dmz_super *sb = zmd->sb[set].sb;
> - struct dmz_dev *dev = zmd->sb[set].dev;
> + struct dmz_super *sb = dsb->sb;
> + struct dmz_dev *dev = dsb->dev;
> unsigned int nr_meta_zones, nr_data_zones;
> u32 crc, stored_crc;
> u64 gen;
> @@ -1034,7 +1035,7 @@ static int dmz_check_sb(struct dmz_metadata *zmd, unsigned int set)
> DMZ_META_VER, zmd->sb_version);
> return -EINVAL;
> }
> - if ((zmd->sb_version < 1) && (set == 2)) {
> + if ((zmd->sb_version < 1) && tertiary) {
You can remove the parentheses around zmd->sb_version < 1 here.
An shouldn't it be "zmd->sb_version <= 1" here ? That already was in the
previous round of updates and did not detect any problem though. Will check again.
> dmz_dev_err(dev, "Tertiary superblocks are not supported");
> return -EINVAL;
> }
> @@ -1078,7 +1079,7 @@ static int dmz_check_sb(struct dmz_metadata *zmd, unsigned int set)
> return -ENXIO;
> }
>
> - if (set == 2) {
> + if (tertiary) {
> /*
> * Generation number should be 0, but it doesn't
> * really matter if it isn't.
> @@ -1127,14 +1128,13 @@ static int dmz_check_sb(struct dmz_metadata *zmd, unsigned int set)
> /*
> * Read the first or second super block from disk.
> */
> -static int dmz_read_sb(struct dmz_metadata *zmd, unsigned int set)
> +static int dmz_read_sb(struct dmz_metadata *zmd, struct dmz_sb *sb, int set)
> {
> dmz_zmd_debug(zmd, "read superblock set %d dev %s block %llu",
> - set, zmd->sb[set].dev->name,
> - zmd->sb[set].block);
> + set, sb->dev->name, sb->block);
>
> - return dmz_rdwr_block(zmd->sb[set].dev, REQ_OP_READ,
> - zmd->sb[set].block, zmd->sb[set].mblk->page);
> + return dmz_rdwr_block(sb->dev, REQ_OP_READ,
> + sb->block, sb->mblk->page);
> }
>
> /*
> @@ -1162,7 +1162,7 @@ static int dmz_lookup_secondary_sb(struct dmz_metadata *zmd)
> zmd->sb[1].zone = dmz_get(zmd, zone_id + 1);
> zmd->sb[1].dev = zmd->sb[0].dev;
> for (i = 1; i < zmd->nr_rnd_zones; i++) {
> - if (dmz_read_sb(zmd, 1) != 0)
> + if (dmz_read_sb(zmd, &zmd->sb[1], 1) != 0)
> break;
> if (le32_to_cpu(zmd->sb[1].sb->magic) == DMZ_MAGIC)
> return 0;
> @@ -1179,9 +1179,9 @@ static int dmz_lookup_secondary_sb(struct dmz_metadata *zmd)
> }
>
> /*
> - * Read the first or second super block from disk.
> + * Read a super block from disk.
> */
> -static int dmz_get_sb(struct dmz_metadata *zmd, unsigned int set)
> +static int dmz_get_sb(struct dmz_metadata *zmd, struct dmz_sb *sb, int set)
> {
> struct dmz_mblock *mblk;
> int ret;
> @@ -1191,14 +1191,14 @@ static int dmz_get_sb(struct dmz_metadata *zmd, unsigned int set)
> if (!mblk)
> return -ENOMEM;
>
> - zmd->sb[set].mblk = mblk;
> - zmd->sb[set].sb = mblk->data;
> + sb->mblk = mblk;
> + sb->sb = mblk->data;
>
> /* Read super block */
> - ret = dmz_read_sb(zmd, set);
> + ret = dmz_read_sb(zmd, sb, set);
> if (ret) {
> dmz_free_mblock(zmd, mblk);
> - zmd->sb[set].mblk = NULL;
> + sb->mblk = NULL;
> return ret;
> }
>
> @@ -1272,13 +1272,13 @@ static int dmz_load_sb(struct dmz_metadata *zmd)
> /* Read and check the primary super block */
> zmd->sb[0].block = dmz_start_block(zmd, zmd->sb[0].zone);
> zmd->sb[0].dev = dmz_zone_to_dev(zmd, zmd->sb[0].zone);
> - ret = dmz_get_sb(zmd, 0);
> + ret = dmz_get_sb(zmd, &zmd->sb[0], 0);
> if (ret) {
> dmz_dev_err(zmd->sb[0].dev, "Read primary super block failed");
> return ret;
> }
>
> - ret = dmz_check_sb(zmd, 0);
> + ret = dmz_check_sb(zmd, &zmd->sb[0], false);
>
> /* Read and check secondary super block */
> if (ret == 0) {
> @@ -1291,7 +1291,7 @@ static int dmz_load_sb(struct dmz_metadata *zmd)
> }
> zmd->sb[1].block = dmz_start_block(zmd, zmd->sb[1].zone);
> zmd->sb[1].dev = zmd->sb[0].dev;
> - ret = dmz_get_sb(zmd, 1);
> + ret = dmz_get_sb(zmd, &zmd->sb[1], 1);
> } else
> ret = dmz_lookup_secondary_sb(zmd);
>
> @@ -1300,7 +1300,7 @@ static int dmz_load_sb(struct dmz_metadata *zmd)
> return ret;
> }
>
> - ret = dmz_check_sb(zmd, 1);
> + ret = dmz_check_sb(zmd, &zmd->sb[1], false);
> if (ret == 0)
> sb_good[1] = true;
>
> @@ -1345,18 +1345,35 @@ static int dmz_load_sb(struct dmz_metadata *zmd)
> "Using super block %u (gen %llu)",
> zmd->mblk_primary, zmd->sb_gen);
>
> - if ((zmd->sb_version > 1) && zmd->sb[2].zone) {
> - zmd->sb[2].block = dmz_start_block(zmd, zmd->sb[2].zone);
> - zmd->sb[2].dev = dmz_zone_to_dev(zmd, zmd->sb[2].zone);
> - ret = dmz_get_sb(zmd, 2);
> - if (ret) {
> - dmz_dev_err(zmd->sb[2].dev,
> - "Read tertiary super block failed");
> - return ret;
> + if (zmd->sb_version > 1) {
> + int i;
> + struct dmz_sb *sb;
> +
> + sb = kzalloc(sizeof(struct dmz_sb), GFP_KERNEL);
> + if (!sb)
> + return -ENOMEM;
> + for (i = 1; i < zmd->nr_devs; i++) {
> + sb->block = 0;
> + sb->zone = dmz_get(zmd, zmd->dev[i].zone_offset);
> + sb->dev = &zmd->dev[i];
> + if (!dmz_is_meta(sb->zone)) {
> + dmz_dev_err(sb->dev,
> + "Tertiary super block zone %u not marked as metadata zone",
> + sb->zone->id);
> + return -EINVAL;
Mem leaking sb here...
> + }
> + ret = dmz_get_sb(zmd, sb, i + 1);
> + if (ret) {
> + dmz_dev_err(sb->dev,
> + "Read tertiary super block failed");
> + return ret;
Here too.
> + }
> + ret = dmz_check_sb(zmd, sb, true);
> + dmz_free_mblock(zmd, sb->mblk);
> + if (ret == -EINVAL)
> + return ret;
And here.
> }
> - ret = dmz_check_sb(zmd, 2);
> - if (ret == -EINVAL)
> - return ret;
> + kfree(sb);
> }
> return 0;
> }
> @@ -1415,12 +1432,15 @@ static int dmz_init_zone(struct blk_zone *blkz, unsigned int num, void *data)
> zmd->sb[0].zone = zone;
> }
> }
> - if (zmd->nr_devs > 1 && !zmd->sb[2].zone) {
> - /* Tertiary superblock zone */
> - zmd->sb[2].zone = zone;
> + if (zmd->nr_devs > 1 && num == 0) {
> + /*
> + * Tertiary superblock zones are always at the
> + * start of the zoned devices, so mark them
> + * as metadata zone.
> + */
> + set_bit(DMZ_META, &zone->flags);
> }
> }
> -
> return 0;
> }
>
> @@ -2858,16 +2878,6 @@ int dmz_ctr_metadata(struct dmz_dev *dev, int num_dev,
> }
> set_bit(DMZ_META, &zone->flags);
> }
> - if (zmd->sb[2].zone) {
> - zone = dmz_get(zmd, zmd->sb[2].zone->id);
> - if (!zone) {
> - dmz_zmd_err(zmd,
> - "Tertiary metadata zone not present");
> - ret = -ENXIO;
> - goto err;
> - }
> - set_bit(DMZ_META, &zone->flags);
> - }
> /* Load mapping table */
> ret = dmz_load_mapping(zmd);
> if (ret)
>
--
Damien Le Moal
Western Digital Research
next prev parent reply other threads:[~2020-05-28 2:54 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-27 6:22 [PATCHv2 00/15] dm-zoned: multi-device support Hannes Reinecke
2020-05-27 6:22 ` [PATCH 01/15] dm-zoned: add debugging message for reading superblocks Hannes Reinecke
2020-05-27 6:22 ` [PATCH 02/15] dm-zoned: secondary superblock must reside on the same devices than primary superblock Hannes Reinecke
2020-05-27 6:22 ` [PATCH 03/15] dm-zoned: improve logging messages for reclaim Hannes Reinecke
2020-05-27 6:22 ` [PATCH 04/15] dm-zoned: add a 'reserved' zone flag Hannes Reinecke
2020-05-27 6:22 ` [PATCH 05/15] dm-zoned: convert to xarray Hannes Reinecke
2020-05-28 2:46 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 06/15] dm-zoned: temporary superblock for tertiary devices Hannes Reinecke
2020-05-28 2:54 ` Damien Le Moal [this message]
2020-05-27 6:22 ` [PATCH 07/15] dm-zoned: add device pointer to struct dm_zone Hannes Reinecke
2020-05-28 2:57 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 08/15] dm-zoned: add metadata pointer to struct dmz_dev Hannes Reinecke
2020-05-27 6:22 ` [PATCH 09/15] dm-zoned: allocate dm devices dynamically Hannes Reinecke
2020-05-28 2:59 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 10/15] dm-zoned: per-device reclaim Hannes Reinecke
2020-05-28 3:19 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 11/15] dm-zoned: move random and sequential zones into struct dmz_dev Hannes Reinecke
2020-05-28 3:25 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 12/15] dm-zoned: support arbitrary number of devices Hannes Reinecke
2020-05-28 4:04 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 13/15] dm-zoned: allocate zone by device index Hannes Reinecke
2020-05-28 4:08 ` Damien Le Moal
2020-05-29 15:48 ` Hannes Reinecke
2020-05-31 8:54 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 14/15] dm-zoned: select reclaim zone based on " Hannes Reinecke
2020-05-28 4:11 ` Damien Le Moal
2020-05-27 6:22 ` [PATCH 15/15] dm-zoned: prefer full zones for reclaim Hannes Reinecke
2020-05-28 4:16 ` Damien Le Moal
2020-05-28 5:15 ` Damien Le Moal
2020-05-27 13:41 ` [PATCHv2 00/15] dm-zoned: multi-device support Mike Snitzer
2020-05-28 4:20 ` Damien Le Moal
2020-05-28 8:24 ` Damien Le Moal
2020-06-02 11:09 [PATCHv4 00/15] dm-zoned: multiple drive support Hannes Reinecke
2020-06-02 11:09 ` [PATCH 06/15] dm-zoned: temporary superblock for tertiary devices Hannes Reinecke
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=CY4PR04MB37510485B74E55A17B4106BDE78E0@CY4PR04MB3751.namprd04.prod.outlook.com \
--to=damien.lemoal@wdc.com \
--cc=dm-devel@redhat.com \
--cc=hare@suse.de \
--cc=snitzer@redhat.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.