On 9/26/22 1:20 PM, Pankaj Raghav wrote: > On 2022-09-26 18:32, Jens Axboe wrote: >> On 9/26/22 8:43 AM, Christoph Hellwig wrote: >>> On Mon, Sep 26, 2022 at 08:40:54AM -0600, Jens Axboe wrote: >>>> On 9/26/22 8:37 AM, Christoph Hellwig wrote: >>>>> On Sun, Sep 25, 2022 at 08:53:46PM +0200, Pankaj Raghav wrote: >>>>>> Modify blk_mq_plug() to allow plugging only for read operations in zoned >>>>>> block devices as there are alternative IO paths in the linux block >>>>>> layer which can end up doing a write via driver private requests in >>>>>> sequential write zones. >>>>> >>>>> We should be able to plug for all operations that are not >>>>> REQ_OP_ZONE_APPEND just fine. >>>> >>>> Agree, I think we just want to make this about someone doing a series >>>> of appends. If you mix-and-match with passthrough you will have a bad >>>> time anyway. >>> >>> Err, sorry - what I wrote about is compelte garbage. I initially >>> wanted to say you can plug for REQ_OP_ZONE_APPEND just fine, and then >>> realized that we also want various other ones that have the write bit >>> set batched. So I suspect we really want to explicitly check for >>> REQ_OP_WRITE here. >> >> My memory was a bit hazy, since we have separate ops for the driver >> in/out, I think just checking for REQ_OP_WRITE is indeed the right >> choice. That's the single case we need to care about. >> > Ah. You are right. I missed it as well. There is even a comment from > Christoph: > > * - if the least significant bit is set transfers are TO the device > * - if the least significant bit is not set transfers are FROM the device > > I guess the second patch should be enough to apply plugging when > applicable for uring_cmd based nvme passthrough requests. Do we even need the 2nd patch? If we're just doing passthrough for the blk_execute_nowait() API, then the condition should never trigger? If so, then it would be a cleanup just to ensure we're using a consistent API for getting the plug, which may be worthwhile to do separately for sure. -- Jens Axboe
