[PATCH 4/6] nvme: zns: Add support for power_of_2 emulation to NVMe ZNS devices

[Damien Le Moal]

On 3/11/22 05:35, Luis Chamberlain wrote:
> On Thu, Mar 10, 2022 at 06:43:47AM +0900, Damien Le Moal wrote:
>> On 3/9/22 23:33, Pankaj Raghav wrote:
>>> On 2022-03-09 05:04, Damien Le Moal wrote:
>>>> So for a power of 2 zone sized device, you are forcing an indirect call,
>>>> always. Not acceptable. What is the point of that po2_zone_emu boolean
>>>> you added to the queue ?
>>> This is a good point and we had a discussion about this internally.
>>> Initially I had something like this:
>>> if (!blk_queue_is_po2_zone_emu(disk))
>>> 	return sector >> (ns->lba_shift - SECTOR_SHIFT);
>>> else
>>> 	return __nvme_sect_to_lba_po2(ns, sec);
>>
>> No need for the else.
> 
> If true then great.

If true ? The else is clearly not needed here. One less line of code.

> 
>>> But @Luis indicated that it was better to set an op which comes at a cost of indirection
>>> instead of having a runtime check with a if/else in the **hot path**. The code also looks
>>> more clear with having an op.
>>
>> The indirect call using a function pointer makes the code obscure. And
>> the cost of that call is far greater and always present compared to the
>> CPU branch prediction which will luckily avoid most of the time taking
>> the wrong branch of an if.
> 
> The goal was to ensure no performance impact, and given a hot path
> was involved and we simply cannot microbench append as there is no
> way / API to do that, we can't be sure. But if you are certain that
> there is no perf impact, it would be wonderful to live without it.

Use zonefs. It uses zone append.

> 
> Thanks for the suggestion and push!
> 
>   Luis


-- 
Damien Le Moal
Western Digital Research