Re: [PATCH] nilfs2: fix block-offset handling in iomap reads

[Ryusuke Konishi]

On Fri, Jul 31, 2026 at 5:38 PM Linmao Li wrote:
>
> nilfs_iomap_begin() looks up the filesystem block containing the requested
> offset and returns the physical address of that block.  However, it sets
> iomap->offset to the original, possibly sub-block, offset while leaving
> iomap->addr at the start of the physical block.
>
> iomap_sector() adds the difference between the I/O position and
> iomap->offset to iomap->addr.  For an O_DIRECT read at offset 512 on a
> filesystem with 4 KiB blocks, the two file offsets are equal and the I/O is
> therefore submitted at the start of the physical block instead of 512 bytes
> into it.  iomap direct I/O permits this alignment when the device logical
> block size is 512 bytes.
>
> Describe mapped and hole extents from the filesystem-block-aligned file
> offset.  This makes the physical and file offsets refer to the same byte.
> It also prevents a one-block hole mapping from extending into the following
> block.
>
> Fixes: b924d8d4e54f ("nilfs2: switch O_DIRECT reads to iomap")
> Signed-off-by: Linmao Li <lilinmao@xxxxxxxxxx>
> ---
>  fs/nilfs2/iomap.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nilfs2/iomap.c b/fs/nilfs2/iomap.c
> index 3ae3bf6ed3686..e130ed63abd9a 100644
> --- a/fs/nilfs2/iomap.c
> +++ b/fs/nilfs2/iomap.c
> @@ -18,6 +18,7 @@ static int nilfs_iomap_begin(struct inode *inode, loff_t offset,
>         struct the_nilfs *nilfs = inode->i_sb->s_fs_info;
>         struct nilfs_inode_info *ii = NILFS_I(inode);
>         sector_t blkoff = offset >> inode->i_blkbits;
> +       loff_t iomap_offset = (loff_t)blkoff << inode->i_blkbits;
>         unsigned int maxblocks;
>         __u64 blknum = 0;
>         int ret;
> @@ -50,15 +51,15 @@ static int nilfs_iomap_begin(struct inode *inode, loff_t offset,
>         if (ret == -ENOENT) {
>                 iomap->type = IOMAP_HOLE;
>                 iomap->addr = IOMAP_NULL_ADDR;
> -               iomap->offset = offset;
> -               iomap->length = min_t(loff_t, length, i_blocksize(inode));
> +               iomap->offset = iomap_offset;
> +               iomap->length = i_blocksize(inode);
>                 return 0;
>         } else if (ret < 0)
>                 return ret;
>
>         iomap->bdev = inode->i_sb->s_bdev;
> -       iomap->offset = offset;
> -       iomap->length = min_t(loff_t, length, (loff_t)ret << inode->i_blkbits);
> +       iomap->offset = iomap_offset;
> +       iomap->length = (loff_t)ret << inode->i_blkbits;
>         iomap->addr = (loff_t)blknum << inode->i_blkbits;
>         iomap->type = IOMAP_MAPPED;
>         iomap->flags = IOMAP_F_MERGED;
>
> base-commit: d5c57d9bf03516d625521cd6cf5acfd93e3e63c7
> --
> 2.25.1

Hi Linmao,

Since the O_DIRECT iomap conversion patch series is currently under
review and subject to revision, please post your finding as comments
on the original patch thread in the future, rather than sending a
standalone patch with a premature Fixes tag.

Hi Viacheslav,
I believe the point being made is that a misalignment occurs because
iomap->addr is adjusted to the block boundary, whereas iomap->offset
is not.  Could you take a look at his points ?

Thanks,
Ryusuke Konishi