On 2019/04/10 16:48, Dan Carpenter wrote:
> My static checker complains about this line from dmz_get_zoned_device()
>
> aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
>
> The problem is that "aligned_capacity" and "dev->capacity" are sector_t
> type (which is a u64) but blk_queue_zone_sectors(q) returns a u32 so the
> higher 32 bits in "aligned_capacity" are always cleared to zero. This
> patch adds a cast to u64 to address this issue.
>
> Fixes: 114e025968b5 ("dm zoned: ignore last smaller runt zone")
> Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> ---
> v2: In v1 I changed blk_queue_zone_sectors() to return a sector_t type,
> but in v2 I just add a cast. The v2 fix would end up going through
> different maintainers and reviewers so the CC list has grown...
>
> Original discussion: https://marc.info/?l=kernel-janitors&m=155487663405737&w=2
>
> drivers/md/dm-zoned-target.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/dm-zoned-target.c b/drivers/md/dm-zoned-target.c
> index 8865c1709e16..b6cb44fa946d 100644
> --- a/drivers/md/dm-zoned-target.c
> +++ b/drivers/md/dm-zoned-target.c
> @@ -643,7 +643,8 @@ static int dmz_get_zoned_device(struct dm_target *ti, char *path)
>
> q = bdev_get_queue(dev->bdev);
> dev->capacity = i_size_read(dev->bdev->bd_inode) >> SECTOR_SHIFT;
> - aligned_capacity = dev->capacity & ~(blk_queue_zone_sectors(q) - 1);
> + aligned_capacity = dev->capacity &
> + ~((u64)blk_queue_zone_sectors(q) - 1);
sector_t is an u64 only if CONFIG_LBDAF is defined (I think this option is going
away though). Otherwise it is an unsigned long which would be u32 on 32 bits
arch. Not a problem in terms of arithmetic, but why not cast to sector_t directly ?
> if (ti->begin ||
> ((ti->len != dev->capacity) && (ti->len != aligned_capacity))) {
> ti->error = "Partial mapping not supported";
>
--
Damien Le Moal
Western Digital Research
