Message ID | 156174694534.1557952.14317442337573957232.stgit@magnolia (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xfs: further FSSETXATTR cleanups | expand |
On 6/28/19 11:35 AM, Darrick J. Wong wrote: > From: Darrick J. Wong <darrick.wong@oracle.com> > > We have no ability to change S_DAX on an inode, which means that this > flag is purely advisory. Remove all the (broken) code that tried to > change the state since it's cluttering up the code for no good reason. > If the kernel ever gains the ability to change S_DAX on the fly we can > always add this back. > Looks ok to me. Reviewed-by: Allison Collins <allison.henderson@oracle.com> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com> > --- > fs/xfs/xfs_ioctl.c | 76 ---------------------------------------------------- > 1 file changed, 76 deletions(-) > > > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c > index d2526d9070d2..dda681698555 100644 > --- a/fs/xfs/xfs_ioctl.c > +++ b/fs/xfs/xfs_ioctl.c > @@ -1001,12 +1001,6 @@ xfs_diflags_to_linux( > inode->i_flags |= S_NOATIME; > else > inode->i_flags &= ~S_NOATIME; > -#if 0 /* disabled until the flag switching races are sorted out */ > - if (xflags & FS_XFLAG_DAX) > - inode->i_flags |= S_DAX; > - else > - inode->i_flags &= ~S_DAX; > -#endif > } > > static int > @@ -1077,63 +1071,6 @@ xfs_ioctl_setattr_drain_writes( > return inode_drain_writes(inode); > } > > -/* > - * If we are changing DAX flags, we have to ensure the file is clean and any > - * cached objects in the address space are invalidated and removed. This > - * requires us to lock out other IO and page faults similar to a truncate > - * operation. The locks need to be held until the transaction has been committed > - * so that the cache invalidation is atomic with respect to the DAX flag > - * manipulation. > - * > - * The caller must use @join_flags to release the locks which are held on @ip > - * regardless of return value. > - */ > -static int > -xfs_ioctl_setattr_dax_invalidate( > - struct xfs_inode *ip, > - struct fsxattr *fa, > - int *join_flags) > -{ > - struct inode *inode = VFS_I(ip); > - struct super_block *sb = inode->i_sb; > - int error; > - > - /* > - * It is only valid to set the DAX flag on regular files and > - * directories on filesystems where the block size is equal to the page > - * size. On directories it serves as an inherited hint so we don't > - * have to check the device for dax support or flush pagecache. > - */ > - if (fa->fsx_xflags & FS_XFLAG_DAX) { > - if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) > - return -EINVAL; > - if (S_ISREG(inode->i_mode) && > - !bdev_dax_supported(xfs_find_bdev_for_inode(VFS_I(ip)), > - sb->s_blocksize)) > - return -EINVAL; > - } > - > - /* If the DAX state is not changing, we have nothing to do here. */ > - if ((fa->fsx_xflags & FS_XFLAG_DAX) && IS_DAX(inode)) > - return 0; > - if (!(fa->fsx_xflags & FS_XFLAG_DAX) && !IS_DAX(inode)) > - return 0; > - > - if (S_ISDIR(inode->i_mode)) > - return 0; > - > - /* lock, flush and invalidate mapping in preparation for flag change */ > - if (*join_flags == 0) { > - *join_flags = XFS_MMAPLOCK_EXCL | XFS_IOLOCK_EXCL; > - xfs_ilock(ip, *join_flags); > - error = filemap_write_and_wait(inode->i_mapping); > - if (error) > - return error; > - } > - > - return invalidate_inode_pages2(inode->i_mapping); > -} > - > /* > * Set up the transaction structure for the setattr operation, checking that we > * have permission to do so. On success, return a clean transaction and the > @@ -1346,19 +1283,6 @@ xfs_ioctl_setattr( > goto error_free_dquots; > } > > - /* > - * Changing DAX config may require inode locking for mapping > - * invalidation. These need to be held all the way to transaction commit > - * or cancel time, so need to be passed through to > - * xfs_ioctl_setattr_get_trans() so it can apply them to the join call > - * appropriately. > - */ > - code = xfs_ioctl_setattr_dax_invalidate(ip, fa, &join_flags); > - if (code) { > - xfs_iunlock(ip, join_flags); > - goto error_free_dquots; > - } > - > tp = xfs_ioctl_setattr_get_trans(ip, join_flags); > if (IS_ERR(tp)) { > code = PTR_ERR(tp); >
diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c index d2526d9070d2..dda681698555 100644 --- a/fs/xfs/xfs_ioctl.c +++ b/fs/xfs/xfs_ioctl.c @@ -1001,12 +1001,6 @@ xfs_diflags_to_linux( inode->i_flags |= S_NOATIME; else inode->i_flags &= ~S_NOATIME; -#if 0 /* disabled until the flag switching races are sorted out */ - if (xflags & FS_XFLAG_DAX) - inode->i_flags |= S_DAX; - else - inode->i_flags &= ~S_DAX; -#endif } static int @@ -1077,63 +1071,6 @@ xfs_ioctl_setattr_drain_writes( return inode_drain_writes(inode); } -/* - * If we are changing DAX flags, we have to ensure the file is clean and any - * cached objects in the address space are invalidated and removed. This - * requires us to lock out other IO and page faults similar to a truncate - * operation. The locks need to be held until the transaction has been committed - * so that the cache invalidation is atomic with respect to the DAX flag - * manipulation. - * - * The caller must use @join_flags to release the locks which are held on @ip - * regardless of return value. - */ -static int -xfs_ioctl_setattr_dax_invalidate( - struct xfs_inode *ip, - struct fsxattr *fa, - int *join_flags) -{ - struct inode *inode = VFS_I(ip); - struct super_block *sb = inode->i_sb; - int error; - - /* - * It is only valid to set the DAX flag on regular files and - * directories on filesystems where the block size is equal to the page - * size. On directories it serves as an inherited hint so we don't - * have to check the device for dax support or flush pagecache. - */ - if (fa->fsx_xflags & FS_XFLAG_DAX) { - if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode))) - return -EINVAL; - if (S_ISREG(inode->i_mode) && - !bdev_dax_supported(xfs_find_bdev_for_inode(VFS_I(ip)), - sb->s_blocksize)) - return -EINVAL; - } - - /* If the DAX state is not changing, we have nothing to do here. */ - if ((fa->fsx_xflags & FS_XFLAG_DAX) && IS_DAX(inode)) - return 0; - if (!(fa->fsx_xflags & FS_XFLAG_DAX) && !IS_DAX(inode)) - return 0; - - if (S_ISDIR(inode->i_mode)) - return 0; - - /* lock, flush and invalidate mapping in preparation for flag change */ - if (*join_flags == 0) { - *join_flags = XFS_MMAPLOCK_EXCL | XFS_IOLOCK_EXCL; - xfs_ilock(ip, *join_flags); - error = filemap_write_and_wait(inode->i_mapping); - if (error) - return error; - } - - return invalidate_inode_pages2(inode->i_mapping); -} - /* * Set up the transaction structure for the setattr operation, checking that we * have permission to do so. On success, return a clean transaction and the @@ -1346,19 +1283,6 @@ xfs_ioctl_setattr( goto error_free_dquots; } - /* - * Changing DAX config may require inode locking for mapping - * invalidation. These need to be held all the way to transaction commit - * or cancel time, so need to be passed through to - * xfs_ioctl_setattr_get_trans() so it can apply them to the join call - * appropriately. - */ - code = xfs_ioctl_setattr_dax_invalidate(ip, fa, &join_flags); - if (code) { - xfs_iunlock(ip, join_flags); - goto error_free_dquots; - } - tp = xfs_ioctl_setattr_get_trans(ip, join_flags); if (IS_ERR(tp)) { code = PTR_ERR(tp);