On Wed, 2016-04-20 at 13:59 -0700, Christoph Hellwig wrote: > On Fri, Apr 15, 2016 at 12:11:36PM -0400, Jeff Moyer wrote: > > > > > > > > + if (IS_DAX(inode)) { > > > + ret = dax_do_io(iocb, inode, iter, offset, > > > blkdev_get_block, > > > NULL, DIO_SKIP_DIO_COUNT); > > > + if (ret == -EIO && (iov_iter_rw(iter) == WRITE)) > > > + ret_saved = ret; > > > + else > > > + return ret; > > > + } > > > + > > > + ret = __blockdev_direct_IO(iocb, inode, I_BDEV(inode), > > > iter, offset, > > > blkdev_get_block, NULL, > > > NULL, > > > DIO_SKIP_DIO_COUNT); > > > + if (ret < 0 && ret_saved) > > > + return ret_saved; > > > + > > Hmm, did you just break async DIO? I think you did! :) > > __blockdev_direct_IO can return -EIOCBQUEUED, and you've now turned > > that > > into -EIO. Really, I don't see a reason to save that first > > -EIO. The > > same applies to all instances in this patch. > Yes, there is no point in saving the earlier error - just return the > second error all the time. Is it ok to do that? direct_IO might fail with -EINVAL due to misalignment, or -ENOMEM due to some allocation failing, and I thought we should return the original -EIO in such cases so that the application doesn't lose the information that the bad block is actually causing the error. > > E.g. > > ret = dax_io(); > if (dax_need_dio_retry(ret)) > ret = direct_IO(); > ��.n��������+%������w��{.n�����{����n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�