Re: [PATCH v3 2/2] block: use blk_mq_plug() wrapper consistently in the block layer

[Pankaj Raghav <p.raghav@samsung.com>]

On 2022-09-29 15:45, Jens Axboe wrote:
> On 9/29/22 7:41 AM, Pankaj Raghav wrote:
>>>> Either of the changes should not have led to a bug in zoned devices:
>>>>
>>>> - blk_execute_rq_nowait:
>>>>   Only passthrough requests can use this function, and plugging can be
>>>>   performed on those requests in zoned devices. So no issues directly
>>>>   accessing the plug.
>>>>
>>>> - blk_flush_plug in bio_poll:
>>>>   As we don't plug the requests that require a zone lock in the first
>>>>   place, flushing should not have any impact. So no issues directly
>>>>   accessing the plug.
>>>>
>>>> This is just a cleanup patch to use this wrapper to get the plug
>>>> consistently across the block layer.
>>>
>>> While I did suggest to make this consistent and in principle it's
>>> the right thing to do, it also irks me to add extra checks to paths
>>> where we know that it's just extra pointless code. Maybe we can
>>> just comment these two spots? Basically each of the sections above
>>> could just go into the appropriate file.
>>>
>> The checks should go away, and the plug could be inlined by the
>> compiler if we don't have CONFIG_BLK_DEV_ZONED. Otherwise, I do agree
>> with you that it is a pointless check.
> 
> But that's my general complaint with the argument that "it doesn't
> matter if this feature isn't configured" - distros enable features.
> Anything that uses IS_ENABLED() is handy for testing, but assuming it
> all pretty much gets enabled in the distro kernel, it does absolutely
> nothing to help the added overhead. It's something that gets done to
> create the illusion that an added feature CAN have zero core overhead,
> while in reality that's utterly wrong.
> Got it!
>> I am fine with either, though I prefer what this patch is doing. So if
>> you feel strongly against calling the blk_mq_plug function, I can turn
>> this patch into just adding comments as you suggested.
> 
> I think we should. I can pick patch 1 here, and then you can send a
> patch 2 for that when you have time.
> 

I will do it right away as a separate patch! Thanks.