Re: EIO with io_uring O_DIRECT writes on ext4

[Jens Axboe <axboe@kernel.dk>]

On 7/23/19 2:07 PM, Theodore Y. Ts'o wrote:
> On Tue, Jul 23, 2019 at 09:20:05AM -0600, Jens Axboe wrote:
>>
>> I actually think it's XFS that's broken here, it's not passing down
>> the IOCB_NOWAIT -> IOMAP_NOWAIT -> REQ_NOWAIT. This means we lose that
>> important request bit, and we just block instead of triggering the
>> not_supported case.
>>
>> Outside of that, that case needs similar treatment to what I did for
>> the EAGAIN case here:
>>
>> http://git.kernel.dk/cgit/linux-block/commit/?h=for-linus&id=893a1c97205a3ece0cbb3f571a3b972080f3b4c7
>>
>> It was a big mistake to pass back these values in an async fashion,
>> and it also means that some accounting in other drivers are broken
>> as we can get completions without the bio actually being submitted.
> 
> Hmmm, I had been trying to track down a similar case with virtio-scsi
> on top of LVM, using a Google Compute Engine VM.  In that case,
> xfstests generic/471 was failing with EIO, when it would pass just
> *fine* when I was using KVM with a virtio-scsi device w/o LVM.
> 
> So it sounds like that what's going on is if the device driver (or
> LVM, or anything else in the storage stack) needs to do a blocking
> memory allocation, and NOWAIT is requested, we'll end up returning EIO
> because an asynchronous error is getting reported, where as if we
> could return it synchronously, the file system could properly return
> EOPNOTSUP.  Am I understanding you correctly?

Yes, that's exactly right. The EAGAIN/EOPNOTSUPP for blocking reasons
should be returned sync, so ext4/xfs can return that error as well. This
enables us to punt the IO appropriately to a workqueue. It should NOT
result in being translated into an EIO to the application.

Working on this change right now...

> I guess there's a separate question hiding here, which is whether
> there's a way to allow dm-linear or dm-crypt to handle NOWAIT requests
> without needing to block.

That's certainly the next step. Right now we just guard this with
queue_is_mq(), but in reality it should be a queue flag so that stacking
drivers can opt in when they have been vetted/changed to support NOWAIT
properly.

But wading through this stuff is leaving me a little disappointed in the
level of NOWAIT support we have right now. It seems mostly half-assed
and there are plenty of cases where we don't really do the right thing.
I'll try and work through all that, to the extent possible.

-- 
Jens Axboe