On Thu, Jul 22, 2021 at 06:52:33PM -0700, Eric Biggers wrote:
> I am trying to do this, but unfortunately I don't see a way to make it work
> correctly in all cases.
>
> The main problem is that when iomap_dio_rw() returns an error (other than
> -EIOCBQUEUED), there is no way to know whether ->end_io() has been called or
> not. This is because iomap_dio_rw() can fail either early, before "starting"
> the I/O (in which case ->end_io() won't have been called), or later, after
> "starting" the I/O (in which case ->end_io() will have been called). Note that
> this can't be worked around by checking whether the iov_iter has been advanced
> or not, since a failure could occur between "starting" the I/O and the iov_iter
> being advanced for the first time.
>
> Would you be receptive to adding a ->begin_io() callback to struct iomap_dio_ops
> in order to allow filesystems to maintain counters like this?
I think we can triviall fix this by using the slightly lower level
__iomap_dio_rw API. Incremental patch to my previous one below:
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index 4fed90cc1462..11844bd0cb7a 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -4243,6 +4243,7 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
struct f2fs_inode_info *fi = F2FS_I(inode);
const loff_t pos = iocb->ki_pos;
const size_t count = iov_iter_count(to);
+ struct iomap_dio *dio;
ssize_t ret;
if (count == 0)
@@ -4260,8 +4261,13 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
}
inc_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
- ret = iomap_dio_rw(iocb, to, &f2fs_iomap_ops, &f2fs_iomap_dio_ops, 0);
-
+ dio = __iomap_dio_rw(iocb, to, &f2fs_iomap_ops, &f2fs_iomap_dio_ops, 0);
+ if (IS_ERR_OR_NULL(dio)) {
+ dec_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
+ ret = PTR_ERR_OR_ZERO(dio);
+ } else {
+ ret = iomap_dio_complete(dio);
+ }
up_read(&fi->i_gc_rwsem[READ]);
file_accessed(file);
@@ -4271,8 +4277,6 @@ static ssize_t f2fs_dio_read_iter(struct kiocb *iocb, struct iov_iter *to)
else if (ret == -EIOCBQUEUED)
f2fs_update_iostat(F2FS_I_SB(inode), APP_DIRECT_READ_IO,
count - iov_iter_count(to));
- else
- dec_page_count(F2FS_I_SB(inode), F2FS_DIO_READ);
out:
trace_f2fs_direct_IO_exit(inode, pos, count, READ, ret);
return ret;
