| Message ID | 319766d2fd03bd47f773d320577f263f68ba67a1.1729825985.git.ritesh.list@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ext4: Add atomic write support for DIO | expand |
On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: > Let's validate using generic_atomic_write_valid() in > ext4_file_write_iter() if the write request has IOCB_ATOMIC set. > > Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > --- > fs/ext4/file.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/fs/ext4/file.c b/fs/ext4/file.c > index f14aed14b9cf..b06c5d34bbd2 100644 > --- a/fs/ext4/file.c > +++ b/fs/ext4/file.c > @@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from) > if (IS_DAX(inode)) > return ext4_dax_write_iter(iocb, from); > #endif > + > + if (iocb->ki_flags & IOCB_ATOMIC) { > + size_t len = iov_iter_count(from); > + int ret; > + > + if (!IS_ALIGNED(len, EXT4_SB(inode->i_sb)->fs_awu_min) || > + len > EXT4_SB(inode->i_sb)->fs_awu_max) > + return -EINVAL; this looks ok, but the IS_ALIGNED() check looks odd. I am not sure why you don't just check that fs_awu_max >= len >= fs_awu_min > + > + ret = generic_atomic_write_valid(iocb, from); > + if (ret) > + return ret; > + } > + > if (iocb->ki_flags & IOCB_DIRECT) > return ext4_dio_write_iter(iocb, from); > else
John Garry <john.g.garry@oracle.com> writes: > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: >> Let's validate using generic_atomic_write_valid() in >> ext4_file_write_iter() if the write request has IOCB_ATOMIC set. >> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> --- >> fs/ext4/file.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/fs/ext4/file.c b/fs/ext4/file.c >> index f14aed14b9cf..b06c5d34bbd2 100644 >> --- a/fs/ext4/file.c >> +++ b/fs/ext4/file.c >> @@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from) >> if (IS_DAX(inode)) >> return ext4_dax_write_iter(iocb, from); >> #endif >> + >> + if (iocb->ki_flags & IOCB_ATOMIC) { >> + size_t len = iov_iter_count(from); >> + int ret; >> + >> + if (!IS_ALIGNED(len, EXT4_SB(inode->i_sb)->fs_awu_min) || >> + len > EXT4_SB(inode->i_sb)->fs_awu_max) >> + return -EINVAL; > > this looks ok, but the IS_ALIGNED() check looks odd. I am not sure why > you don't just check that fs_awu_max >= len >= fs_awu_min > I guess this was just a stricter check. But we anyways have power_of_2 and other checks in generic_atomic_write_valid(). So it does not matter. I can change this in v2. Thanks! >> + >> + ret = generic_atomic_write_valid(iocb, from); >> + if (ret) >> + return ret; >> + } >> + >> if (iocb->ki_flags & IOCB_DIRECT) >> return ext4_dio_write_iter(iocb, from); >> else -ritesh
On Fri, Oct 25, 2024 at 04:03:02PM +0530, Ritesh Harjani wrote: > John Garry <john.g.garry@oracle.com> writes: > > > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: > >> Let's validate using generic_atomic_write_valid() in > >> ext4_file_write_iter() if the write request has IOCB_ATOMIC set. > >> > >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> > >> --- > >> fs/ext4/file.c | 14 ++++++++++++++ > >> 1 file changed, 14 insertions(+) > >> > >> diff --git a/fs/ext4/file.c b/fs/ext4/file.c > >> index f14aed14b9cf..b06c5d34bbd2 100644 > >> --- a/fs/ext4/file.c > >> +++ b/fs/ext4/file.c > >> @@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from) > >> if (IS_DAX(inode)) > >> return ext4_dax_write_iter(iocb, from); > >> #endif > >> + > >> + if (iocb->ki_flags & IOCB_ATOMIC) { > >> + size_t len = iov_iter_count(from); > >> + int ret; > >> + > >> + if (!IS_ALIGNED(len, EXT4_SB(inode->i_sb)->fs_awu_min) || > >> + len > EXT4_SB(inode->i_sb)->fs_awu_max) > >> + return -EINVAL; > > > > this looks ok, but the IS_ALIGNED() check looks odd. I am not sure why > > you don't just check that fs_awu_max >= len >= fs_awu_min > > > > I guess this was just a stricter check. But we anyways have power_of_2 > and other checks in generic_atomic_write_valid(). So it does not matter. > > I can change this in v2. Also please fix the weird indenting in the if test: if (len < EXT4_SB(inode->i_sb)->fs_awu_min) || len > EXT4_SB(inode->i_sb)->fs_awu_max) return -EINVAL; --D > Thanks! > > >> + > >> + ret = generic_atomic_write_valid(iocb, from); > >> + if (ret) > >> + return ret; > >> + } > >> + > >> if (iocb->ki_flags & IOCB_DIRECT) > >> return ext4_dio_write_iter(iocb, from); > >> else > > -ritesh >
"Darrick J. Wong" <djwong@kernel.org> writes: > On Fri, Oct 25, 2024 at 04:03:02PM +0530, Ritesh Harjani wrote: >> John Garry <john.g.garry@oracle.com> writes: >> >> > On 25/10/2024 04:45, Ritesh Harjani (IBM) wrote: >> >> Let's validate using generic_atomic_write_valid() in >> >> ext4_file_write_iter() if the write request has IOCB_ATOMIC set. >> >> >> >> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> >> >> --- >> >> fs/ext4/file.c | 14 ++++++++++++++ >> >> 1 file changed, 14 insertions(+) >> >> >> >> diff --git a/fs/ext4/file.c b/fs/ext4/file.c >> >> index f14aed14b9cf..b06c5d34bbd2 100644 >> >> --- a/fs/ext4/file.c >> >> +++ b/fs/ext4/file.c >> >> @@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from) >> >> if (IS_DAX(inode)) >> >> return ext4_dax_write_iter(iocb, from); >> >> #endif >> >> + >> >> + if (iocb->ki_flags & IOCB_ATOMIC) { >> >> + size_t len = iov_iter_count(from); >> >> + int ret; >> >> + >> >> + if (!IS_ALIGNED(len, EXT4_SB(inode->i_sb)->fs_awu_min) || >> >> + len > EXT4_SB(inode->i_sb)->fs_awu_max) >> >> + return -EINVAL; >> > >> > this looks ok, but the IS_ALIGNED() check looks odd. I am not sure why >> > you don't just check that fs_awu_max >= len >= fs_awu_min >> > >> >> I guess this was just a stricter check. But we anyways have power_of_2 >> and other checks in generic_atomic_write_valid(). So it does not matter. >> >> I can change this in v2. > > Also please fix the weird indenting in the if test: > > if (len < EXT4_SB(inode->i_sb)->fs_awu_min) || > len > EXT4_SB(inode->i_sb)->fs_awu_max) > return -EINVAL; > > --D Got it! -ritesh > >> Thanks! >> >> >> + >> >> + ret = generic_atomic_write_valid(iocb, from); >> >> + if (ret) >> >> + return ret; >> >> + } >> >> + >> >> if (iocb->ki_flags & IOCB_DIRECT) >> >> return ext4_dio_write_iter(iocb, from); >> >> else >> >> -ritesh >>
diff --git a/fs/ext4/file.c b/fs/ext4/file.c index f14aed14b9cf..b06c5d34bbd2 100644 --- a/fs/ext4/file.c +++ b/fs/ext4/file.c @@ -692,6 +692,20 @@ ext4_file_write_iter(struct kiocb *iocb, struct iov_iter *from) if (IS_DAX(inode)) return ext4_dax_write_iter(iocb, from); #endif + + if (iocb->ki_flags & IOCB_ATOMIC) { + size_t len = iov_iter_count(from); + int ret; + + if (!IS_ALIGNED(len, EXT4_SB(inode->i_sb)->fs_awu_min) || + len > EXT4_SB(inode->i_sb)->fs_awu_max) + return -EINVAL; + + ret = generic_atomic_write_valid(iocb, from); + if (ret) + return ret; + } + if (iocb->ki_flags & IOCB_DIRECT) return ext4_dio_write_iter(iocb, from); else
Let's validate using generic_atomic_write_valid() in ext4_file_write_iter() if the write request has IOCB_ATOMIC set. Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> --- fs/ext4/file.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)