[dm-devel] [PATCH v2] dm zoned: Silence a static checker warning

[Damien Le Moal]

On 2019/04/10 17:03, Dan Carpenter wrote:
> On Wed, Apr 10, 2019 at 07:56:14AM +0000, Damien Le Moal wrote:
>> 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 at oracle.com>
>>> ---
>>> 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 ?
>>
> 
> I would have prefered to do that but I didn't have strong feelings
> either way.  I am always slight annoyed when people don't just copy and
> paste my code when I tell them how to fix a patch so I decided to go
> with your version...  :P

I proposed u64 cast without checking sector_t definition.
After doing that, I liked your sector_t cast more than my own misinformed
proposal :)

-- 
Damien Le Moal
Western Digital Research