| Message ID | 7c4779f1f0c8ead30f660a2cfbdf4d7cc08e405a.1729825985.git.ritesh.list@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: Add atomic write support for DIO | expand |
On Fri, Oct 25, 2024 at 09:15:53AM +0530, Ritesh Harjani (IBM) wrote: > iomap will not return -ENOTBLK in case of dio atomic writes. But let's > also add a WARN_ON_ONCE and return -EIO as a safety net. > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > --- > fs/ext4/file.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index f9516121a036..af6ebd0ac0d6 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -576,8 +576,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) > iomap_ops = &ext4_iomap_overwrite_ops; > ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, > dio_flags, NULL, 0); > - if (ret == -ENOTBLK) > + if (ret == -ENOTBLK) { > ret = 0; > + /* > + * iomap will never return -ENOTBLK if write fails for atomic > + * write. But let's just add a safety net. I think it can if the pagecache invalidation fails, so you really do need the safety net. I suspect that the xfs version of this series needs it too, though it may have fallen out? Reviewed-by: Darrick J. Wong <djwong@kernel.org> --D > + */ > + if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC)) > + ret = -EIO; > + } > + > if (extend) { > /* > * We always perform extending DIO write synchronously so by > -- > 2.46.0 > >
"Darrick J. Wong" <djwong@kernel.org> writes: > On Fri, Oct 25, 2024 at 09:15:53AM +0530, Ritesh Harjani (IBM) wrote: >> iomap will not return -ENOTBLK in case of dio atomic writes. But let's >> also add a WARN_ON_ONCE and return -EIO as a safety net. >> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> --- >> fs/ext4/file.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/fs/ext4/file.c b/fs/ext4/file.c >> index f9516121a036..af6ebd0ac0d6 100644 >> --- a/fs/ext4/file.c >> +++ b/fs/ext4/file.c >> @@ -576,8 +576,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) >> iomap_ops = &ext4_iomap_overwrite_ops; >> ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, >> dio_flags, NULL, 0); >> - if (ret == -ENOTBLK) >> + if (ret == -ENOTBLK) { >> ret = 0; >> + /* >> + * iomap will never return -ENOTBLK if write fails for atomic >> + * write. But let's just add a safety net. > > I think it can if the pagecache invalidation fails, so you really do > need the safety net. Ah, right! So in that case I should remove WARN_ON_ONCE and correct the comment too. > I suspect that the xfs version of this series > needs it too, though it may have fallen out? > I think so yes. Looks like it got missed. > Reviewed-by: Darrick J. Wong <djwong@kernel.org> > > --D > Thanks for pointing that out. -ritesh >> + */ >> + if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC)) >> + ret = -EIO; >> + } >> + >> if (extend) { >> /* >> * We always perform extending DIO write synchronously so by >> -- >> 2.46.0 >> >>
diff --git a/fs/ext4/file.c b/fs/ext4/file.c index f9516121a036..af6ebd0ac0d6 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -576,8 +576,16 @@ static ssize_t ext4_dio_write_iter(struct kiocb *iocb, struct iov_iter *from) iomap_ops = &ext4_iomap_overwrite_ops; ret = iomap_dio_rw(iocb, from, iomap_ops, &ext4_dio_write_ops, dio_flags, NULL, 0); - if (ret == -ENOTBLK) + if (ret == -ENOTBLK) { ret = 0; + /* + * iomap will never return -ENOTBLK if write fails for atomic + * write. But let's just add a safety net. + */ + if (WARN_ON_ONCE(iocb->ki_flags & IOCB_ATOMIC)) + ret = -EIO; + } + if (extend) { /* * We always perform extending DIO write synchronously so by
iomap will not return -ENOTBLK in case of dio atomic writes. But let's also add a WARN_ON_ONCE and return -EIO as a safety net. Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> --- fs/ext4/file.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)