On 12/1/23 3:49 PM, Keith Busch wrote: > On Fri, Dec 01, 2023 at 11:42:53AM -0700, Keith Busch wrote: >> On Fri, Dec 01, 2023 at 04:13:45PM +0530, Kanchan Joshi wrote: >>> On 12/1/2023 3:23 AM, Keith Busch wrote: >>>> From: Keith Busch<kbusch@xxxxxxxxxx> >>> >>> This causes a regression (existed in previous version too). >>> System freeze on issuing single read/write io that used to work fine >>> earlier: >>> fio -iodepth=1 -rw=randread -ioengine=io_uring_cmd -cmd_type=nvme >>> -bs=4096 -numjobs=1 -size=4096 -filename=/dev/ng0n1 -md_per_io_size=8 >>> -name=pt >>> >>> This is because we pin one bvec during submission, but unpin 4 on >>> completion. bio_integrity_unpin_bvec() uses bip->bip_max_vcnt, which is >>> set to 4 (equal to BIO_INLINE_VECS) in this case. >>> >>> To use bip_max_vcnt the way this series uses, we need below patch/fix: >> >> Thanks for the catch! Earlier versions of this series was capped by the >> byte count rather than the max_vcnt value, so the inline condition >> didn't matter before. I think your update looks good. I'll double check >> what's going on with my custom tests to see why it didn't see this >> problem. > > Got it: I was using ioctl instead of iouring. ioctl doesn't set > REQ_ALLOC_CACHE, so we don't get a bio_set in bio_integrity_alloc(), and > that makes inline_vecs set similiar to what your diff does. > > Jens already applied the latest series for the next merge. We can append > this or fold atop, or back it out and we can rework it for another > version. No rush; for your patch: I folded this into the original to avoid the breakage, even if it wasn't a huge concern for this particular issue. But it's close enough to merging, figured we may as well do that rather than have a fixup patch. Please check the end result, both for-next and for-6.8/block are updated now. -- Jens Axboe
