On Thu 19-07-18 14:23:53, Martin Wilck wrote: > On Thu, 2018-07-19 at 12:45 +0200, Jan Kara wrote: > > Secondly, I don't think it is good to discard error from > > bio_iov_iter_get_pages() here and just submit partial IO. It will > > again > > lead to part of IO being done as direct and part attempted to be done > > as > > buffered. Also the "slow" direct IO path in __blkdev_direct_IO() > > behaves > > differently - it aborts and returns error if bio_iov_iter_get_pages() > > ever > > returned error. IMO we should do the same here. > > Well, it aborts the loop, but then (in the sync case) it still waits > for the already submitted IOs to finish. Here, too, I'd find it more > logical to return the number of successfully transmitted bytes rather > than an error code. In the async case, the submitted bios are left in > place, and will probably sooner or later finish, changing iocb->ki_pos. Well, both these behaviors make sense, just traditionally (defined by our implementation) DIO returns error even if part of IO has actually been successfully submitted. Making a userspace visible change like you suggest thus has to be very carefully analyzed and frankly I don't think it's worth the bother. > I'm actually not quite certain if that's correct. In the sync case, it > causes the already-performed IO to be done again, buffered. In the > async case, it it may even cause two IOs for the same range to be in > flight at the same time ... ? It doesn't cause IO to be done again. Look at __generic_file_write_iter(). If generic_file_direct_write() returned error, we immediately return error as well without retrying buffered IO. We only retry buffered IO for partial (or 0) return value. Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
