Re: [PATCH 2/2] nilfs2: switch O_DIRECT reads to iomap

On Sat, Jul 25, 2026 at 7:28=E2=80=AFAM Viacheslav Dubeyko  wrote:
>
> Wire the read-only nilfs_iomap_ops added in the previous patch into
> the O_DIRECT read path, and eliminate blockdev_direct_IO() from
> nilfs2 entirely:
>
>  - nilfs_file_open() now sets FMODE_CAN_ODIRECT explicitly, since
>    permission to open the file O_DIRECT was previously implied by
>    aops->direct_IO being non-NULL.
>  - nilfs_file_read_iter() dispatches O_DIRECT reads to iomap_dio_rw()
>    using nilfs_iomap_ops; everything else still goes through
>    generic_file_read_iter() as before.
>  - nilfs_file_write_iter() strips IOCB_DIRECT and falls through to
>    generic_file_write_iter()'s ordinary buffered path. NILFS2 cannot
>    perform true direct I/O writes: new blocks are delay-allocated and
>    only given a real disk address by the segment constructor, which
>    works on buffer_head lists, not iomap. This reproduces today's
>    actual behavior: the old nilfs_direct_IO() already just returned 0
>    for WRITE.
>  - nilfs_direct_IO() and the .direct_IO callback on nilfs_aops are
>    removed.
>  - drop the unnecessary "select LEGACY_DIRECT_IO" from Kconfig
>    in favor of "select FS_IOMAP".
>
> Signed-off-by: Viacheslav Dubeyko <[email protected]>
> cc: Christoph Hellwig <[email protected]>
> cc: Ryusuke Konishi <[email protected]>
> cc: [email protected]
> cc: [email protected]
> ---
>  fs/nilfs2/Kconfig |  2 +-
>  fs/nilfs2/file.c  | 40 +++++++++++++++++++++++++++++++++++++---
>  fs/nilfs2/inode.c | 13 -------------
>  3 files changed, 38 insertions(+), 17 deletions(-)
>
> diff --git a/fs/nilfs2/Kconfig b/fs/nilfs2/Kconfig
> index 7dae168e346e..0a5ace60e6ab 100644
> --- a/fs/nilfs2/Kconfig
> +++ b/fs/nilfs2/Kconfig
> @@ -3,7 +3,7 @@ config NILFS2_FS
>         tristate "NILFS2 file system support"
>         select BUFFER_HEAD
>         select CRC32
> -       select LEGACY_DIRECT_IO
> +       select FS_IOMAP
>         help
>           NILFS2 is a log-structured file system (LFS) supporting continu=
ous
>           snapshotting.  In addition to versioning capability of the enti=
re
> diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
> index f93b68c4877c..ad2e87c049c9 100644
> --- a/fs/nilfs2/file.c
> +++ b/fs/nilfs2/file.c
> @@ -10,9 +10,12 @@
>  #include <linux/fs.h>
>  #include <linux/filelock.h>
>  #include <linux/mm.h>
> +#include <linux/uio.h>
> +#include <linux/iomap.h>
>  #include <linux/writeback.h>
>  #include "nilfs.h"
>  #include "segment.h"
> +#include "iomap.h"
>
>  int nilfs_sync_file(struct file *file, loff_t start, loff_t end, int dat=
async)
>  {
> @@ -133,20 +136,51 @@ static int nilfs_file_mmap_prepare(struct vm_area_d=
esc *desc)
>         return 0;
>  }
>
> +static int nilfs_file_open(struct inode *inode, struct file *file)
> +{
> +       file->f_mode |=3D FMODE_CAN_ODIRECT;
> +       return generic_file_open(inode, file);
> +}
> +
> +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);
> +}
> +
> +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.
> +        */
> +       if (iocb->ki_flags & IOCB_DIRECT)
> +               iocb->ki_flags &=3D ~IOCB_DIRECT;
> +
> +       return generic_file_write_iter(iocb, from);
> +}
> +
>  /*
>   * We have mostly NULL's here: the current defaults are ok for
>   * the nilfs filesystem.
>   */
>  const struct file_operations nilfs_file_operations =3D {
>         .llseek         =3D generic_file_llseek,
> -       .read_iter      =3D generic_file_read_iter,
> -       .write_iter     =3D generic_file_write_iter,
> +       .read_iter      =3D nilfs_file_read_iter,
> +       .write_iter     =3D nilfs_file_write_iter,
>         .unlocked_ioctl =3D nilfs_ioctl,
>  #ifdef CONFIG_COMPAT
>         .compat_ioctl   =3D nilfs_compat_ioctl,
>  #endif /* CONFIG_COMPAT */
>         .mmap_prepare   =3D nilfs_file_mmap_prepare,
> -       .open           =3D generic_file_open,
> +       .open           =3D nilfs_file_open,
>         /* .release     =3D nilfs_release_file, */
>         .fsync          =3D nilfs_sync_file,
>         .splice_read    =3D filemap_splice_read,
> diff --git a/fs/nilfs2/inode.c b/fs/nilfs2/inode.c
> index 51f7e125a311..f4a9d9ea9c3f 100644
> --- a/fs/nilfs2/inode.c
> +++ b/fs/nilfs2/inode.c
> @@ -257,18 +257,6 @@ static int nilfs_write_end(const struct kiocb *iocb,
>         return err ? : copied;
>  }
>
> -static ssize_t
> -nilfs_direct_IO(struct kiocb *iocb, struct iov_iter *iter)
> -{
> -       struct inode *inode =3D file_inode(iocb->ki_filp);
> -
> -       if (iov_iter_rw(iter) =3D=3D WRITE)
> -               return 0;
> -
> -       /* Needs synchronization with the cleaner */
> -       return blockdev_direct_IO(iocb, inode, iter, nilfs_get_block);
> -}
> -
>  const struct address_space_operations nilfs_aops =3D {
>         .read_folio             =3D nilfs_read_folio,
>         .writepages             =3D nilfs_writepages,
> @@ -277,7 +265,6 @@ const struct address_space_operations nilfs_aops =3D =
{
>         .write_begin            =3D nilfs_write_begin,
>         .write_end              =3D nilfs_write_end,
>         .invalidate_folio       =3D block_invalidate_folio,
> -       .direct_IO              =3D nilfs_direct_IO,
>         .migrate_folio          =3D buffer_migrate_folio_norefs,
>         .is_partially_uptodate  =3D block_is_partially_uptodate,
>  };
> --
> 2.43.0

The conversion looks good to me.
(Note: I haven't exhaustively checked for any iomap-specific caveats.)

I verified the build and basic operations in several local
environments, and the changes have been working without any critical
issues so far.

Acked-by: Ryusuke Konishi <[email protected]>

Thanks,
Ryusuke Konishi