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
