Re: [PATCH 1/6] imsm: avoid overflows for disks over 1TB

[NeilBrown <neilb@xxxxxxx>]

On Thu, 29 Mar 2012 12:43:00 +0000 "Czarnowska, Anna"
<anna.czarnowska@xxxxxxxxx> wrote:


> +static unsigned long long total_blocks(struct imsm_disk *disk)
> +{
> +	if (disk == 0)
> +		return 0;

I changed this and some others to
        if (disk == NULL)
                return 0;


> +	return join_u32(disk->total_blocks_lo, disk->total_blocks_hi);
> +}
> +
> +static unsigned long long pba_of_lba0(struct imsm_map *map)
> +{
> +	if (map == 0)
> +		return 0;
> +	return join_u32(map->pba_of_lba0_lo, map->pba_of_lba0_hi);
> +}
> +
> +static unsigned long long blocks_per_member(struct imsm_map *map)
> +{
> +	if (map == 0)
> +		return 0;
> +	return join_u32(map->blocks_per_member_lo, map->blocks_per_member_hi);
> +}
> +
> +static unsigned long long num_data_stripes(struct imsm_map *map)
> +{
> +	if (map == 0)
> +		return 0;
> +	return join_u32(map->num_data_stripes_lo, map->num_data_stripes_hi);
> +}
> +
> +static void set_total_blocks(struct imsm_disk *disk, unsigned long long n)
> +{
> +	split_ull(n, &disk->total_blocks_lo, &disk->total_blocks_hi);
> +}
> +
> +static void set_pba_of_lba0(struct imsm_map *map, unsigned long long n)
> +{
> +	split_ull(n, &map->pba_of_lba0_lo, &map->pba_of_lba0_hi);
> +}
> +
> +static void set_blocks_per_member(struct imsm_map *map, unsigned long long n)
> +{
> +	split_ull(n, &map->blocks_per_member_lo, &map->blocks_per_member_hi);
> +}
> +
> +static void set_num_data_stripes(struct imsm_map *map, unsigned long long n)
> +{
> +	split_ull(n, &map->num_data_stripes_lo, &map->num_data_stripes_hi);
> +}
> +
>  static struct extent *get_extents(struct intel_super *super, struct dl *dl)
>  {
>  	/* find a list of used extents on the given physical device */
> @@ -897,8 +964,8 @@ static struct extent *get_extents(struct intel_super *super, struct dl *dl)
>  		struct imsm_map *map = get_imsm_map(dev, MAP_0);
>  
>  		if (get_imsm_disk_slot(map, dl->index) >= 0) {
> -			e->start = __le32_to_cpu(map->pba_of_lba0);
> -			e->size = __le32_to_cpu(map->blocks_per_member);
> +			e->start = pba_of_lba0(map);
> +			e->size = blocks_per_member(map);
>  			e++;
>  		}
>  	}
> @@ -911,10 +978,9 @@ static struct extent *get_extents(struct intel_super *super, struct dl *dl)
>  	 */
>  	if (memberships) {
>  		struct extent *last = &rv[memberships - 1];
> -		__u32 remainder;
> +		unsigned long long remainder;
>  
> -		remainder = __le32_to_cpu(dl->disk.total_blocks) - 
> -			    (last->start + last->size);
> +		remainder = total_blocks(&dl->disk) - (last->start + last->size);
>  		/* round down to 1k block to satisfy precision of the kernel
>  		 * 'size' interface
>  		 */
> @@ -925,7 +991,7 @@ static struct extent *get_extents(struct intel_super *super, struct dl *dl)
>  		if (reservation > remainder)
>  			reservation = remainder;
>  	}
> -	e->start = __le32_to_cpu(dl->disk.total_blocks) - reservation;
> +	e->start = total_blocks(&dl->disk) - reservation;
>  	e->size = 0;
>  	return rv;
>  }
> @@ -954,7 +1020,7 @@ static __u32 imsm_reserved_sectors(struct intel_super *super, struct dl *dl)
>  	for (i = 0; e[i].size; i++)
>  		continue;
>  
> -	rv = __le32_to_cpu(dl->disk.total_blocks) - e[i].start;
> +	rv = total_blocks(&dl->disk) - e[i].start;
>  
>  	free(e);
>  
> @@ -984,7 +1050,8 @@ static __u32 imsm_min_reserved_sectors(struct intel_super *super)
>  {
>  	struct extent *e;
>  	int i;
> -	__u32 min_active, remainder;
> +	unsigned long long min_active;
> +	__u32 remainder;

I'm a bit concerned that 'remainder' is still a '__u32'.  It is calculated as
the difference of two 'unsigned long long'....
I'll take the patch as is, but you might like to fix this up.

Applied.

Thanks,
NeilBrown

Attachment: signature.asc
Description: PGP signature