Re: [PATCH 05/15] dm-zoned: convert to xarray

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2020/05/27 15:23, Hannes Reinecke wrote:
> The zones array is getting really large, and large arrays
> tend to wreak havoc with the CPU caches.
> So convert it to xarray to become more cache friendly.
> 
> Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
> ---
>  drivers/md/dm-zoned-metadata.c | 120 ++++++++++++++++++++++++++++++-----------
>  1 file changed, 88 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
> index 0982ab1758a6..839f9078806d 100644
> --- a/drivers/md/dm-zoned-metadata.c
> +++ b/drivers/md/dm-zoned-metadata.c
> @@ -172,7 +172,7 @@ struct dmz_metadata {
>  	unsigned int		nr_chunks;
>  
>  	/* Zone information array */
> -	struct dm_zone		*zones;
> +	struct xarray		zones;
>  
>  	struct dmz_sb		sb[3];
>  	unsigned int		mblk_primary;
> @@ -327,6 +327,30 @@ unsigned int dmz_nr_unmap_seq_zones(struct dmz_metadata *zmd)
>  	return atomic_read(&zmd->unmap_nr_seq);
>  }
>  
> +static struct dm_zone *dmz_get(struct dmz_metadata *zmd, unsigned int zone_id)
> +{
> +	return xa_load(&zmd->zones, zone_id);
> +}
> +
> +static struct dm_zone *dmz_insert(struct dmz_metadata *zmd,
> +				  unsigned int zone_id)
> +{
> +	struct dm_zone *zone = kzalloc(sizeof(struct dm_zone), GFP_KERNEL);
> +
> +	if (!zone)
> +		return ERR_PTR(-ENOMEM);
> +
> +	if (xa_insert(&zmd->zones, zone_id, zone, GFP_KERNEL))
> +		return ERR_PTR(-EBUSY);
> +
> +	INIT_LIST_HEAD(&zone->link);
> +	atomic_set(&zone->refcount, 0);
> +	zone->id = zone_id;
> +	zone->chunk = DMZ_MAP_UNMAPPED;
> +
> +	return zone;
> +}
> +
>  const char *dmz_metadata_label(struct dmz_metadata *zmd)
>  {
>  	return (const char *)zmd->label;
> @@ -1122,6 +1146,7 @@ static int dmz_lookup_secondary_sb(struct dmz_metadata *zmd)
>  {
>  	unsigned int zone_nr_blocks = zmd->zone_nr_blocks;
>  	struct dmz_mblock *mblk;
> +	unsigned int zone_id = zmd->sb[0].zone->id;
>  	int i;
>  
>  	/* Allocate a block */
> @@ -1134,16 +1159,15 @@ static int dmz_lookup_secondary_sb(struct dmz_metadata *zmd)
>  
>  	/* Bad first super block: search for the second one */
>  	zmd->sb[1].block = zmd->sb[0].block + zone_nr_blocks;
> -	zmd->sb[1].zone = zmd->sb[0].zone + 1;
> +	zmd->sb[1].zone = dmz_get(zmd, zone_id + 1);
>  	zmd->sb[1].dev = zmd->sb[0].dev;
> -	for (i = 0; i < zmd->nr_rnd_zones - 1; i++) {
> +	for (i = 1; i < zmd->nr_rnd_zones; i++) {
>  		if (dmz_read_sb(zmd, 1) != 0)
>  			break;
> -		if (le32_to_cpu(zmd->sb[1].sb->magic) == DMZ_MAGIC) {
> -			zmd->sb[1].zone += i;
> +		if (le32_to_cpu(zmd->sb[1].sb->magic) == DMZ_MAGIC)
>  			return 0;
> -		}
>  		zmd->sb[1].block += zone_nr_blocks;
> +		zmd->sb[1].zone = dmz_get(zmd, zone_id + i);
>  	}
>  
>  	dmz_free_mblock(zmd, mblk);
> @@ -1259,8 +1283,12 @@ static int dmz_load_sb(struct dmz_metadata *zmd)
>  	/* Read and check secondary super block */
>  	if (ret == 0) {
>  		sb_good[0] = true;
> -		if (!zmd->sb[1].zone)
> -			zmd->sb[1].zone = zmd->sb[0].zone + zmd->nr_meta_zones;
> +		if (!zmd->sb[1].zone) {
> +			unsigned int zone_id =
> +				zmd->sb[0].zone->id + zmd->nr_meta_zones;
> +
> +			zmd->sb[1].zone = dmz_get(zmd, zone_id);
> +		}
>  		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);
> @@ -1341,7 +1369,11 @@ static int dmz_init_zone(struct blk_zone *blkz, unsigned int num, void *data)
>  	struct dmz_metadata *zmd = data;
>  	struct dmz_dev *dev = zmd->nr_devs > 1 ? &zmd->dev[1] : &zmd->dev[0];
>  	int idx = num + dev->zone_offset;
> -	struct dm_zone *zone = &zmd->zones[idx];
> +	struct dm_zone *zone;
> +
> +	zone = dmz_insert(zmd, idx);
> +	if (IS_ERR(zone))
> +		return PTR_ERR(zone);
>  
>  	if (blkz->len != zmd->zone_nr_sectors) {
>  		if (zmd->sb_version > 1) {
> @@ -1353,11 +1385,6 @@ static int dmz_init_zone(struct blk_zone *blkz, unsigned int num, void *data)
>  		return -ENXIO;
>  	}
>  
> -	INIT_LIST_HEAD(&zone->link);
> -	atomic_set(&zone->refcount, 0);
> -	zone->id = idx;
> -	zone->chunk = DMZ_MAP_UNMAPPED;
> -
>  	switch (blkz->type) {
>  	case BLK_ZONE_TYPE_CONVENTIONAL:
>  		set_bit(DMZ_RND, &zone->flags);
> @@ -1397,18 +1424,17 @@ static int dmz_init_zone(struct blk_zone *blkz, unsigned int num, void *data)
>  	return 0;
>  }
>  
> -static void dmz_emulate_zones(struct dmz_metadata *zmd, struct dmz_dev *dev)
> +static int dmz_emulate_zones(struct dmz_metadata *zmd, struct dmz_dev *dev)
>  {
>  	int idx;
>  	sector_t zone_offset = 0;
>  
>  	for(idx = 0; idx < dev->nr_zones; idx++) {
> -		struct dm_zone *zone = &zmd->zones[idx];
> +		struct dm_zone *zone;
>  
> -		INIT_LIST_HEAD(&zone->link);
> -		atomic_set(&zone->refcount, 0);
> -		zone->id = idx;
> -		zone->chunk = DMZ_MAP_UNMAPPED;
> +		zone = dmz_insert(zmd, idx);
> +		if (IS_ERR(zone))
> +			return PTR_ERR(zone);
>  		set_bit(DMZ_CACHE, &zone->flags);
>  		zone->wp_block = 0;
>  		zmd->nr_cache_zones++;
> @@ -1420,6 +1446,7 @@ static void dmz_emulate_zones(struct dmz_metadata *zmd, struct dmz_dev *dev)
>  		}
>  		zone_offset += zmd->zone_nr_sectors;
>  	}
> +	return 0;
>  }
>  
>  /*
> @@ -1427,8 +1454,15 @@ static void dmz_emulate_zones(struct dmz_metadata *zmd, struct dmz_dev *dev)
>   */
>  static void dmz_drop_zones(struct dmz_metadata *zmd)
>  {
> -	kfree(zmd->zones);
> -	zmd->zones = NULL;
> +	int idx;
> +
> +	for(idx = 0; idx < zmd->nr_zones; idx++) {
> +		struct dm_zone *zone = xa_load(&zmd->zones, idx);
> +
> +		kfree(zone);
> +		xa_erase(&zmd->zones, idx);
> +	}
> +	xa_destroy(&zmd->zones);
>  }
>  
>  /*
> @@ -1460,20 +1494,25 @@ static int dmz_init_zones(struct dmz_metadata *zmd)
>  		DMERR("(%s): No zones found", zmd->devname);
>  		return -ENXIO;
>  	}
> -	zmd->zones = kcalloc(zmd->nr_zones, sizeof(struct dm_zone), GFP_KERNEL);
> -	if (!zmd->zones)
> -		return -ENOMEM;
> +	xa_init(&zmd->zones);
>  
>  	DMDEBUG("(%s): Using %zu B for zone information",
>  		zmd->devname, sizeof(struct dm_zone) * zmd->nr_zones);
>  
>  	if (zmd->nr_devs > 1) {
> -		dmz_emulate_zones(zmd, &zmd->dev[0]);
> +		ret = dmz_emulate_zones(zmd, &zmd->dev[0]);
> +		if (ret < 0) {
> +			DMDEBUG("(%s): Failed to emulate zones, error %d",
> +				zmd->devname, ret);
> +			dmz_drop_zones(zmd);
> +			return ret;
> +		}
> +
>  		/*
>  		 * Primary superblock zone is always at zone 0 when multiple
>  		 * drives are present.
>  		 */
> -		zmd->sb[0].zone = &zmd->zones[0];
> +		zmd->sb[0].zone = dmz_get(zmd, 0);
>  
>  		zoned_dev = &zmd->dev[1];
>  	}
> @@ -1576,11 +1615,6 @@ static int dmz_handle_seq_write_err(struct dmz_metadata *zmd,
>  	return 0;
>  }
>  
> -static struct dm_zone *dmz_get(struct dmz_metadata *zmd, unsigned int zone_id)
> -{
> -	return &zmd->zones[zone_id];
> -}
> -
>  /*
>   * Reset a zone write pointer.
>   */
> @@ -1662,6 +1696,11 @@ static int dmz_load_mapping(struct dmz_metadata *zmd)
>  		}
>  
>  		dzone = dmz_get(zmd, dzone_id);
> +		if (!dzone) {
> +			dmz_zmd_err(zmd, "Chunk %u mapping: data zone %u not present",
> +				    chunk, dzone_id);
> +			return -EIO;
> +		}
>  		set_bit(DMZ_DATA, &dzone->flags);
>  		dzone->chunk = chunk;
>  		dmz_get_zone_weight(zmd, dzone);
> @@ -1685,6 +1724,11 @@ static int dmz_load_mapping(struct dmz_metadata *zmd)
>  		}
>  
>  		bzone = dmz_get(zmd, bzone_id);
> +		if (!bzone) {
> +			dmz_zmd_err(zmd, "Chunk %u mapping: buffer zone %u not present",
> +				    chunk, bzone_id);
> +			return -EIO;
> +		}
>  		if (!dmz_is_rnd(bzone) && !dmz_is_cache(bzone)) {
>  			dmz_zmd_err(zmd, "Chunk %u mapping: invalid buffer zone %u",
>  				    chunk, bzone_id);
> @@ -1715,6 +1759,8 @@ static int dmz_load_mapping(struct dmz_metadata *zmd)
>  	 */
>  	for (i = 0; i < zmd->nr_zones; i++) {
>  		dzone = dmz_get(zmd, i);
> +		if (!dzone)
> +			continue;
>  		if (dmz_is_meta(dzone))
>  			continue;
>  		if (dmz_is_offline(dzone))
> @@ -1978,6 +2024,10 @@ struct dm_zone *dmz_get_chunk_mapping(struct dmz_metadata *zmd, unsigned int chu
>  	} else {
>  		/* The chunk is already mapped: get the mapping zone */
>  		dzone = dmz_get(zmd, dzone_id);
> +		if (!dzone) {
> +			dzone = ERR_PTR(-EIO);
> +			goto out;
> +		}
>  		if (dzone->chunk != chunk) {
>  			dzone = ERR_PTR(-EIO);
>  			goto out;
> @@ -2794,6 +2844,12 @@ int dmz_ctr_metadata(struct dmz_dev *dev, int num_dev,
>  	/* Set metadata zones starting from sb_zone */
>  	for (i = 0; i < zmd->nr_meta_zones << 1; i++) {
>  		zone = dmz_get(zmd, zmd->sb[0].zone->id + i);
> +		if (!zone) {
> +			dmz_zmd_err(zmd,
> +				    "metadata zone %u not present", i);
> +			ret = -ENXIO;
> +			goto err;
> +		}
>  		if (!dmz_is_rnd(zone) && !dmz_is_cache(zone)) {
>  			dmz_zmd_err(zmd,
>  				    "metadata zone %d is not random", i);
> 

Looks good.

Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>

-- 
Damien Le Moal
Western Digital Research



--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel




[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux