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

[Luis Chamberlain <mcgrof@kernel.org>]

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.

> > 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.

Thanks for the suggestion and push!

  Luis