> } else if (flags & IOMAP_WRITE) {
> u64 size;
> +
> + if (flags & IOMAP_DIRECT)
> + goto out;
> +
Maybe add a comment here on why you don't allow block allocations for
direct I/O.
> + if (flags & IOMAP_DIRECT) {
> + ret = gfs2_iomap_get(inode, pos, length, flags, iomap,
> &mp);
> + release_metapath(&mp);
> + if (iomap->type != IOMAP_MAPPED)
> + ret = -ENOTBLK;
> + } else {
> + ret = gfs2_iomap_begin_write(inode, pos, length, flags,
> iomap);
> + }
A couple too long lines.
> } else {
> ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
> release_metapath(&mp);
But shouldn't the direct I/O code try to reuse this part anyway?
E.g. something like:
if ((flags & (IOMAP_WRITE | IOMAP_DIRECT)) == IOMAP_WRITE)) {
ret = gfs2_iomap_begin_write(inode, pos, length, flags, iomap);
} else {
ret = gfs2_iomap_get(inode, pos, length, flags, iomap, &mp);
release_metapath(&mp);
if ((flags & IOMAP_WRITE) && iomap->type != IOMAP_MAPPED)
ret = -ENOTBLK;
> + /* fall back to buffered I/O for stuffed files */
> + ret = -ENOTBLK;
> + if (gfs2_is_stuffed(ip))
> + goto out;
I think we can handle stuffed files in the direct I/O code trivially
by copying out the inline data in the iomap. It would be great to
just handle this instead of adding fallbacks.
> + /* Silently fall back to buffered I/O for stuffed files */
> + if (gfs2_is_stuffed(ip))
> + goto out;
Same here.
> +static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> + ssize_t ret;
> +
> + if (iocb->ki_flags & IOCB_DIRECT) {
> + ret = gfs2_file_direct_read(iocb, to);
> + if (likely(ret != -ENOTBLK))
> + goto out;
return ret;
> + iocb->ki_flags &= ~IOCB_DIRECT;
> + }
> + ret = generic_file_read_iter(iocb, to);
> +out:
> + return ret;
return generic_file_read_iter(iocb, to);
