Re: [PATCH v2 2/2] nilfs2: switch O_DIRECT to iomap based operations

Hi Viacheslav,

On Sat, Aug 8, 2026 at 9:35 AM Viacheslav Dubeyko wrote:
...
> +static ssize_t nilfs_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
> +{
> +       if (iocb->ki_flags & IOCB_DIRECT) {
> +               return iomap_dio_rw(iocb, to, &nilfs_iomap_ops,
> +                                   NULL, 0, NULL, 0);
> +       } else
> +               return generic_file_read_iter(iocb, to);
> +}

Among the tests that used to pass before the iomap conversion, I found that
running xfstests generic/418 and generic/465 now fails with a "broken bmap"
error, causing the filesystem to remount read-only.

This happens because iomap_dio_rw() is called without holding the shared
inode lock.  To prevent this race, I think iomap_dio_rw() should be
protected as follows:

        struct inode *inode = file_inode(iocb->ki_filp);
        ssize_t ret;

        if (iocb->ki_flags & IOCB_DIRECT) {
                inode_lock_shared(inode);
                ret = iomap_dio_rw(iocb, to, &nilfs_iomap_ops,
                                NULL, 0, NULL, 0);
                inode_unlock_shared(inode);
        } else {
                ret = generic_file_read_iter(iocb, to);
        }

        return ret;

Adding this locking resolved the failures in both generic/418 and
generic/465 in my tests.

> +
> +static ssize_t nilfs_file_write_iter(struct kiocb *iocb, struct iov_iter *from)
> +{
> +       /*
> +        * NILFS2 cannot perform true direct I/O writes: new blocks are
> +        * delay-allocated and are only given a real disk address when
> +        * the segment constructor writes them out as part of a log,
> +        * which works directly on buffer_head lists rather than
> +        * through iomap. Fall back to the ordinary buffered write path
> +        * for O_DIRECT writes.
> +        */

Also, regarding this comment, following Christoph's suggestion, how
about simplifying it as follows?

        /*
         * NILFS2 lacks direct I/O write support; fall back to buffered writes.
         */

Thanks,
Ryusuke Konishi