| Message ID | 20240304130428.13026-15-john.g.garry@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block atomic writes for XFS | expand |
On Mon, Mar 04, 2024 at 01:04:28PM +0000, John Garry wrote: > For when an inode is enabled for atomic writes, set FMODE_CAN_ATOMIC_WRITE > flag. We check for direct I/O and also check that the bdev can actually > support atomic writes. > > We rely on the block layer to reject atomic writes which exceed the bdev > request_queue limits, so don't bother checking any such thing here. > > Signed-off-by: John Garry <john.g.garry@oracle.com> > --- > fs/xfs/xfs_file.c | 21 +++++++++++++++++++++ > 1 file changed, 21 insertions(+) > > diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c > index cecc5428fd7c..e63851be6c15 100644 > --- a/fs/xfs/xfs_file.c > +++ b/fs/xfs/xfs_file.c > @@ -1234,6 +1234,25 @@ xfs_file_remap_range( > return remapped > 0 ? remapped : ret; > } > > +static bool xfs_file_open_can_atomicwrite( > + struct inode *inode, > + struct file *file) > +{ > + struct xfs_inode *ip = XFS_I(inode); > + struct xfs_buftarg *target = xfs_inode_buftarg(ip); > + > + if (!(file->f_flags & O_DIRECT)) > + return false; > + > + if (!xfs_inode_atomicwrites(ip)) > + return false; > + > + if (!bdev_can_atomic_write(target->bt_bdev)) > + return false; Again, this is static blockdev information - the inode atomic write flag should not be set if the bdev cannot do atomic writes. It should be checked at mount time - the filesystem probably should only mount read-only if it is configured to allow atomic writes and the underlying blockdev does not support atomic writes... -Dave.
On 06/03/2024 21:33, Dave Chinner wrote: >> +static bool xfs_file_open_can_atomicwrite( >> + struct inode *inode, >> + struct file *file) >> +{ >> + struct xfs_inode *ip = XFS_I(inode); >> + struct xfs_buftarg *target = xfs_inode_buftarg(ip); >> + >> + if (!(file->f_flags & O_DIRECT)) >> + return false; >> + >> + if (!xfs_inode_atomicwrites(ip)) >> + return false; >> + >> + if (!bdev_can_atomic_write(target->bt_bdev)) >> + return false; > Again, this is static blockdev information - the inode atomic write > flag should not be set if the bdev cannot do atomic writes. It > should be checked at mount time ok > - the filesystem probably should > only mount read-only if it is configured to allow atomic writes and > the underlying blockdev does not support atomic writes... Let me know if you really would like to see that change also. It does seem a bit drastic, considering we can just disallow atomic writes. Thanks, John
diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c index cecc5428fd7c..e63851be6c15 100644 --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -1234,6 +1234,25 @@ xfs_file_remap_range( return remapped > 0 ? remapped : ret; } +static bool xfs_file_open_can_atomicwrite( + struct inode *inode, + struct file *file) +{ + struct xfs_inode *ip = XFS_I(inode); + struct xfs_buftarg *target = xfs_inode_buftarg(ip); + + if (!(file->f_flags & O_DIRECT)) + return false; + + if (!xfs_inode_atomicwrites(ip)) + return false; + + if (!bdev_can_atomic_write(target->bt_bdev)) + return false; + + return true; +} + STATIC int xfs_file_open( struct inode *inode, @@ -1243,6 +1262,8 @@ xfs_file_open( return -EIO; file->f_mode |= FMODE_NOWAIT | FMODE_BUF_RASYNC | FMODE_BUF_WASYNC | FMODE_DIO_PARALLEL_WRITE | FMODE_CAN_ODIRECT; + if (xfs_file_open_can_atomicwrite(inode, file)) + file->f_mode |= FMODE_CAN_ATOMIC_WRITE; return generic_file_open(inode, file); }
For when an inode is enabled for atomic writes, set FMODE_CAN_ATOMIC_WRITE flag. We check for direct I/O and also check that the bdev can actually support atomic writes. We rely on the block layer to reject atomic writes which exceed the bdev request_queue limits, so don't bother checking any such thing here. Signed-off-by: John Garry <john.g.garry@oracle.com> --- fs/xfs/xfs_file.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)