Re: [PATCH 12/15] btrfs: Use iomap_dio_rw for performing direct I/O writes

Christoph Hellwig
2019-09-05T16:31:42Z

Lots of lines > 80 chars, and various indentation errors, I'm not
going to point them out invdividually.

> ssize_t btrfs_dio_iomap_read(struct kiocb *iocb, struct iov_iter *to)
> @@ -437,7 +536,58 @@ ssize_t btrfs_dio_iomap_read(struct kiocb *iocb, struct iov_iter *to)
> struct inode *inode = file_inode(iocb->ki_filp);
> ssize_t ret;
> inode_lock_shared(inode);
> - ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, NULL);
> + ret = iomap_dio_rw(iocb, to, &btrfs_dio_iomap_ops, &btrfs_dops);

So the read did not previously need the submit callback, but it does
now? That seems a little odd.

> inode_unlock_shared(inode);
> return ret;
> }
> +
> +ssize_t btrfs_dio_iomap_write(struct kiocb *iocb, struct iov_iter *from)

Why not just brfs_dio_write?

> + written = iomap_dio_rw(iocb, from, &btrfs_dio_iomap_ops, &btrfs_dops);
> + if (written < count) {
> + ssize_t done = (written < 0) ? 0 : written;
> + btrfs_delalloc_release_space(inode, data_reserved, pos, count - done,
> + true);

Line > 80 characters.

> +out:
> + if (written > 0 && iocb->ki_pos > i_size_read(inode))
> + i_size_write(inode, iocb->ki_pos);

Odd indentation.

> + return written ? written : err;

But not:

if (!written)
return err;

if (iocb->ki_pos > i_size_read(inode))
i_size_write(inode, iocb->ki_pos);
return written;