| Message ID | 20241019125113.369994-8-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block atomic writes for xfs | expand |
John Garry <john.g.garry@oracle.com> writes: > Validate that an atomic write adheres to length/offset rules. Currently > we can only write a single FS block. > > For an IOCB with IOCB_ATOMIC set to get as far as xfs_file_write_iter(), > FMODE_CAN_ATOMIC_WRITE will need to be set for the file; for this, > ATOMICWRITES flags would also need to be set for the inode. > > Reviewed-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> > Signed-off-by: John Garry <john.g.garry@oracle.com> > --- > fs/xfs/xfs_file.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index b19916b11fd5..1ccbc1eb75c9 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -852,6 +852,20 @@ xfs_file_write_iter( > if (IS_DAX(inode)) > return xfs_file_dax_write(iocb, from); > > + if (iocb->ki_flags & IOCB_ATOMIC) { > + /* > + * Currently only atomic writing of a single FS block is > + * supported. It would be possible to atomic write smaller than > + * a FS block, but there is no requirement to support this. > + * Note that iomap also does not support this yet. > + */ > + if (ocount != ip->i_mount->m_sb.sb_blocksize) > + return -EINVAL; Shouldn't we "return -ENOTSUPP" ? Given we are later going to add support for ocount > sb_blocksize. -ritesh
On 20/10/2024 10:44, Ritesh Harjani (IBM) wrote: >> + if (iocb->ki_flags & IOCB_ATOMIC) { >> + /* >> + * Currently only atomic writing of a single FS block is >> + * supported. It would be possible to atomic write smaller than >> + * a FS block, but there is no requirement to support this. >> + * Note that iomap also does not support this yet. >> + */ >> + if (ocount != ip->i_mount->m_sb.sb_blocksize) >> + return -EINVAL; > Shouldn't we "return -ENOTSUPP" ? > Given we are later going to add support for ocount > sb_blocksize. So far we have been reporting -EINVAL for an invalid atomic write size (according to atomic write unit min and max reported for that inode). -ENOTSUPP is used for times when we just don't support atomic writes, like non-DIO. Thanks, John
John Garry <john.g.garry@oracle.com> writes: > On 20/10/2024 10:44, Ritesh Harjani (IBM) wrote: >>> + if (iocb->ki_flags & IOCB_ATOMIC) { >>> + /* >>> + * Currently only atomic writing of a single FS block is >>> + * supported. It would be possible to atomic write smaller than >>> + * a FS block, but there is no requirement to support this. >>> + * Note that iomap also does not support this yet. >>> + */ >>> + if (ocount != ip->i_mount->m_sb.sb_blocksize) >>> + return -EINVAL; >> Shouldn't we "return -ENOTSUPP" ? >> Given we are later going to add support for ocount > sb_blocksize. > > So far we have been reporting -EINVAL for an invalid atomic write size > (according to atomic write unit min and max reported for that inode). > > -ENOTSUPP is used for times when we just don't support atomic writes, > like non-DIO. > Sure make sense. -ritesh
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index b19916b11fd5..1ccbc1eb75c9 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -852,6 +852,20 @@ xfs_file_write_iter( if (IS_DAX(inode)) return xfs_file_dax_write(iocb, from); + if (iocb->ki_flags & IOCB_ATOMIC) { + /* + * Currently only atomic writing of a single FS block is + * supported. It would be possible to atomic write smaller than + * a FS block, but there is no requirement to support this. + * Note that iomap also does not support this yet. + */ + if (ocount != ip->i_mount->m_sb.sb_blocksize) + return -EINVAL; + ret = generic_atomic_write_valid(iocb, from); + if (ret) + return ret; + } + if (iocb->ki_flags & IOCB_DIRECT) { /* * Allow a directio write to fall back to a buffered