[dm-devel] [PATCH 03/12] dm-zoned: use on-stack superblock for tertiary devices
Hannes Reinecke
hare at suse.de
Mon May 25 07:41:20 UTC 2020
On 5/25/20 4:09 AM, Damien Le Moal wrote:
> On 2020/05/23 0:39, Hannes Reinecke wrote:
>> Checking the teriary superblock just consists of validating UUIDs,
>
> s/teriary/tertiary
>
>> crcs, and the generation number; it doesn't have contents which
>> would be required during the actual operation.
>> So we should use an on-stack superblock and avoid having to store
>> it together with the 'real' superblocks.
>
> ...a temoprary in-memory superblock allocation...
>
> The entire structure should not be on stack... see below.
>
>>
>> Signed-off-by: Hannes Reinecke <hare at suse.de>
>> ---
>> drivers/md/dm-zoned-metadata.c | 98 +++++++++++++++++++++++-------------------
>> 1 file changed, 53 insertions(+), 45 deletions(-)
>>
>> diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
>> index 3da6702bb1ae..b70a988fa771 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;
>> @@ -995,10 +995,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;
>> @@ -1015,7 +1016,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) {
>> dmz_dev_err(dev, "Tertiary superblocks are not supported");
>> return -EINVAL;
>> }
>> @@ -1059,7 +1060,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.
>> @@ -1108,13 +1109,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)
>> {
>> DMDEBUG("(%s): read superblock set %d dev %s block %llu",
>> zmd->devname, set, zmd->sb[set].dev->name,
>> zmd->sb[set].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);
>> }
>>
>> /*
>> @@ -1142,7 +1143,7 @@ static int dmz_lookup_secondary_sb(struct dmz_metadata *zmd)
>> zmd->sb[1].zone = xa_load(&zmd->zones, zone_id + 1);
>> zmd->sb[1].dev = dmz_zone_to_dev(zmd, zmd->sb[1].zone);
>> 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;
>> @@ -1160,9 +1161,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;
>> @@ -1172,14 +1173,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;
>> }
>>
>> @@ -1253,13 +1254,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) {
>> @@ -1272,7 +1273,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 = dmz_zone_to_dev(zmd, zmd->sb[1].zone);
>> - ret = dmz_get_sb(zmd, 1);
>> + ret = dmz_get_sb(zmd, &zmd->sb[1], 1);
>> } else
>> ret = dmz_lookup_secondary_sb(zmd);
>>
>> @@ -1281,7 +1282,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;
>>
>> @@ -1326,18 +1327,32 @@ 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;
>> +
>> + for (i = 1; i < zmd->nr_devs; i++) {
>> + struct dmz_sb sb;
>
> I would rather have dmz_get_sb() allocate this struct than have it on stack...
> It is not big, but still. To be symetric, we can add dmz_put_sb() for freeing it.
>
Okay, no big deal.
I'll convert it to allocate one.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare at suse.de +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
More information about the dm-devel
mailing list