Hi David,
On Wed, 2020-11-25 at 22:47 +0100, David Sterba wrote:
> On Tue, Nov 10, 2020 at 08:26:07PM +0900, Naohiro Aota wrote:
> > +int btrfs_get_dev_zone_info(struct btrfs_device *device)
> > +{
> > + struct btrfs_zoned_device_info *zone_info = NULL;
> > + struct block_device *bdev = device->bdev;
> > + sector_t nr_sectors = bdev->bd_part->nr_sects;
> > + sector_t sector = 0;
>
> I'd rather replace the sector_t types with u64. The type is unsigned
> long and does not have the same width on 32/64 bit. The typecasts must
> be used and if not, bugs happen (and happened).
Since kernel 5.2, sector_t is unconditionally defined as u64 in linux/type.h:
typedef u64 sector_t;
CONFIG_LBDAF does not exist anymore.
I am not against using u64 at all, but using sector_t makes it clear what the
unit is for the values at hand.
>
> > + struct blk_zone *zones = NULL;
> > + unsigned int i, nreported = 0, nr_zones;
> > + unsigned int zone_sectors;
> > + int ret;
> > +
> > + if (!bdev_is_zoned(bdev))
> > + return 0;
> > +
> > + if (device->zone_info)
> > + return 0;
> > +
> > + zone_info = kzalloc(sizeof(*zone_info), GFP_KERNEL);
> > + if (!zone_info)
> > + return -ENOMEM;
> > +
> > + zone_sectors = bdev_zone_sectors(bdev);
> > + ASSERT(is_power_of_2(zone_sectors));
>
> As is_power_of_2 works only on longs, this needs to be opencoded as
> there's no unsigned long long version.
--
Damien Le Moal
Western Digital
