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
