Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a55665f1-26ae-c19f-5e2e-cf733e8f7fef@samsung.com>
Date:   Wed, 4 May 2022 10:35:04 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
        Thunderbird/91.8.1
Subject: Re: [PATCH 01/16] block: make blkdev_nr_zones and blk_queue_zone_no
 generic for npo2 zsze
Content-Language: en-US
To:     Bart Van Assche <bvanassche@acm.org>, <jaegeuk@kernel.org>,
        <axboe@kernel.dk>, <snitzer@kernel.org>, <hch@lst.de>,
        <mcgrof@kernel.org>, <naohiro.aota@wdc.com>, <sagi@grimberg.me>,
        <damien.lemoal@opensource.wdc.com>, <dsterba@suse.com>,
        <johannes.thumshirn@wdc.com>
CC:     <linux-kernel@vger.kernel.org>, <linux-btrfs@vger.kernel.org>,
        <clm@fb.com>, <gost.dev@samsung.com>, <chao@kernel.org>,
        <linux-f2fs-devel@lists.sourceforge.net>, <josef@toxicpanda.com>,
        <jonathan.derrick@linux.dev>, <agk@redhat.com>,
        <kbusch@kernel.org>, <kch@nvidia.com>,
        <linux-nvme@lists.infradead.org>, <dm-devel@redhat.com>,
        <jiangbo.365@bytedance.com>, <linux-fsdevel@vger.kernel.org>,
        <matias.bjorling@wdc.com>, <linux-block@vger.kernel.org>
From:   Pankaj Raghav <p.raghav@samsung.com>
In-Reply-To: <3a178153-62c0-e298-ccb0-0edfd41b7ee2@acm.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
CMS-TYPE: 201P
References: <20220427160255.300418-1-p.raghav@samsung.com>
        <CGME20220427160257eucas1p21fb58d0129376a135fdf0b9c2fe88895@eucas1p2.samsung.com>
        <20220427160255.300418-2-p.raghav@samsung.com>
        <3a178153-62c0-e298-ccb0-0edfd41b7ee2@acm.org>
Precedence: bulk

On 2022-05-03 18:37, Bart Van Assche wrote:
>>       sector_t zone_sectors = blk_queue_zone_sectors(disk->queue);
>> +    sector_t capacity = get_capacity(disk);
>>         if (!blk_queue_is_zoned(disk->queue))
>>           return 0;
>> -    return (get_capacity(disk) + zone_sectors - 1) >>
>> ilog2(zone_sectors);
>> +
>> +    if (is_power_of_2(zone_sectors))
>> +        return (capacity + zone_sectors - 1) >>
>> +               ilog2(zone_sectors);
>> +
>> +    return div64_u64(capacity + zone_sectors - 1, zone_sectors);
>>   }
>>   EXPORT_SYMBOL_GPL(blkdev_nr_zones);
> 
> Does anyone need support for more than 4 billion sectors per zone? If
> not, do_div() should be sufficient.
> 
You are absolutely right but blk_queue_zone_sectors explicitly changed
the return type to uint32_t to sector_t in this commit:
'113ab72  block: Fix potential overflow in blk_report_zones()'.

I initially did have a do_div but later changed to div64_u64 to avoid
any implicit down casting even though zone_sectors will be always
limited to an unsigned int.
>> diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
>> index 60d016138997..c4e4c7071b7b 100644
>> --- a/include/linux/blkdev.h
>> +++ b/include/linux/blkdev.h
>> @@ -665,9 +665,15 @@ static inline unsigned int
>> blk_queue_nr_zones(struct request_queue *q)
>>   static inline unsigned int blk_queue_zone_no(struct request_queue *q,
>>                            sector_t sector)
>>   {
>> +    sector_t zone_sectors = blk_queue_zone_sectors(q);
>> +
>>       if (!blk_queue_is_zoned(q))
>>           return 0;
>> -    return sector >> ilog2(q->limits.chunk_sectors);
>> +
>> +    if (is_power_of_2(zone_sectors))
>> +        return sector >> ilog2(zone_sectors);
>> +
>> +    return div64_u64(sector, zone_sectors);
>>   }
> 
> Same comment here.
> 
> Thanks,
> 
> Bart.