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

Viacheslav Dubeyko <[email protected]> Sun, 02 Aug 2026 17:33:18 -0700
Newsgroups org.kernel.vger.linux-nilfs,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Fri, 2026-07-31 at 21:05 +0900, Ryusuke Konishi wrote:
> On Fri, Jul 31, 2026 at 5:38=E2=80=AFPM Linmao Li wrote:
> >=20
> > nilfs_iomap_begin() looks up the filesystem block containing the
> > requested
> > offset and returns the physical address of that block.=C2=A0 However, i=
t
> > sets
> > iomap->offset to the original, possibly sub-block, offset while
> > leaving
> > iomap->addr at the start of the physical block.
> >=20
> > iomap_sector() adds the difference between the I/O position and
> > iomap->offset to iomap->addr.=C2=A0 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.=C2=A0 iomap direct I/O permits this alignment when the device
> > logical
> > block size is 512 bytes.
> >=20
> > Describe mapped and hole extents from the filesystem-block-aligned
> > file
> > offset.=C2=A0 This makes the physical and file offsets refer to the sam=
e
> > byte.
> > It also prevents a one-block hole mapping from extending into the
> > following
> > block.
> >=20
> > Fixes: b924d8d4e54f ("nilfs2: switch O_DIRECT reads to iomap")
> > Signed-off-by: Linmao Li <[email protected]>
> > ---
> > =C2=A0fs/nilfs2/iomap.c | 9 +++++----
> > =C2=A01 file changed, 5 insertions(+), 4 deletions(-)
> >=20
> > 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,
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct the_nilfs *nilfs =3D =
inode->i_sb->s_fs_info;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct nilfs_inode_info *ii =
=3D NILFS_I(inode);
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 sector_t blkoff =3D offset >=
> inode->i_blkbits;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 loff_t iomap_offset =3D (loff_t)b=
lkoff << inode->i_blkbits;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 unsigned int maxblocks;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 __u64 blknum =3D 0;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 int ret;
> > @@ -50,15 +51,15 @@ static int nilfs_iomap_begin(struct inode
> > *inode, loff_t offset,
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (ret =3D=3D -ENOENT) {
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 iomap->type =3D IOMAP_HOLE;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 iomap->addr =3D IOMAP_NULL_ADDR;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 iomap->offset =3D offset;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 iomap->length =3D min_t(loff_t, length,
> > i_blocksize(inode));
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 iomap->offset =3D iomap_offset;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0 iomap->length =3D i_blocksize(inode);
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 return 0;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 } else if (ret < 0)
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=
=A0=C2=A0=C2=A0=C2=A0 return ret;
> >=20
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->bdev =3D inode->i_sb-=
>s_bdev;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->offset =3D offset;
> > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->length =3D min_t(loff_t, l=
ength, (loff_t)ret << inode-
> > >i_blkbits);
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->offset =3D iomap_offset;
> > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->length =3D (loff_t)ret << =
inode->i_blkbits;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->addr =3D (loff_t)blkn=
um << inode->i_blkbits;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->type =3D IOMAP_MAPPED=
;
> > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 iomap->flags =3D IOMAP_F_MER=
GED;
> >=20
> > base-commit: d5c57d9bf03516d625521cd6cf5acfd93e3e63c7
> > --
> > 2.25.1
>=20
> Hi Linmao,
>=20
> 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.
>=20
> 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.=C2=A0 Could you take a look at his points ?
>=20

Hi Ryusuke,

The fix makes sense to me. I am attending FMS 2026 (August 4th - 6th).
So, I don't have time for preparing v2 of the patchset with adding
Linmao's fix during this week. Let me try to find the time for pathset
reworking around the end of this week.

Thanks,
Slava.