Re: [PATCH] block: cleanup and fix batch completion adding conditions

[alan.adamson@oracle.com]

On 2/18/25 11:26 PM, Shinichiro Kawasaki wrote:
> CC+: Alan,
>
> On Feb 13, 2025 / 08:18, Jens Axboe wrote:
>> The conditions for whether or not a request is allowed adding to a
>> completion batch are a bit hard to read, and they also have a few
>> issues. One is that ioerror may indeed be a random value on passthrough,
>> and it's being checked unconditionally of whether or not the given
>> request is a passthrough request or not.
>>
>> Rewrite the conditions to be separate for easier reading, and only check
>> ioerror for non-passthrough requests. This fixes an issue with bio
>> unmapping on passthrough, where it fails getting added to a batch. This
>> both leads to suboptimal performance, and may trigger a potential
>> schedule-under-atomic condition for polled passthrough IO.
>>
>> Fixes: f794f3351f26 ("block: add support for blk_mq_end_request_batch()")
>> Signed-off-by: Jens Axboe <axboe@kernel.dk>
> I observed the blktests test case nvme/039 failure with v6.14-rc3 kernel. I
> bisected and found that this patch in the v6.14-rc3 is the trigger.
>
> The test run output is as follows:
>
> nvme/039 => nvme0n1 (test error logging)                     [failed]
>      runtime  5.378s  ...  5.354s
>      --- tests/nvme/039.out      2024-09-20 11:20:26.405380875 +0900
>      +++ /home/shin/Blktests/blktests/results/nvme0n1/nvme/039.out.bad   2025-02-19 16:13:05.061387179 +0900
>      @@ -1,7 +1,3 @@
>       Running nvme/039
>      - Read(0x2) @ LBA 0, 1 blocks, Unrecovered Read Error (sct 0x2 / sc 0x81) DNR
>      - Read(0x2) @ LBA 0, 1 blocks, Unknown (sct 0x3 / sc 0x75) DNR
>      - Write(0x1) @ LBA 0, 1 blocks, Write Fault (sct 0x2 / sc 0x80) DNR
>        Identify(0x6), Access Denied (sct 0x2 / sc 0x86) DNR cdw10=0x1 cdw11=0x0 cdw12=0x0 cdw13=0x0 cdw14=0x0 cdw15=0x0
>      - Read(0x2), Invalid Command Opcode (sct 0x0 / sc 0x1) DNR cdw10=0x0 cdw11=0x0 cdw12=0x1 cdw13=0x0 cdw14=0x0 cdw15=0x0
>       Test complete
>
>
> The test case does error injection. Test method requires reconsideration to
> adjust to this kernel change, probably. Help for fix will be appreciated.

Not really an issue with the test, rather the error injector is broken. 
I'll investigate.

Alan