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.
