On Sun, Jul 07, 2019 at 03:29:42PM -0700, Allison Collins wrote:
>
>
> On 6/28/19 11:35 AM, Darrick J. Wong wrote:
> > From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> >
> > Refactor the SETFLAGS implementation to use the SETXATTR code directly
> > instead of partially constructing a struct fsxattr and calling bits and
> > pieces of the setxattr code. This reduces code size with no functional
> > change.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_ioctl.c | 48 +++---------------------------------------------
> > 1 file changed, 3 insertions(+), 45 deletions(-)
> >
> >
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 552f18554c48..6f55cd7eb34f 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1490,11 +1490,8 @@ xfs_ioc_setxflags(
> > struct file *filp,
> > void __user *arg)
> > {
> > - struct xfs_trans *tp;
> > struct fsxattr fa;
> > - struct fsxattr old_fa;
> > unsigned int flags;
> > - int join_flags = 0;
> > int error;
> > if (copy_from_user(&flags, arg, sizeof(flags)))
> > @@ -1505,52 +1502,13 @@ xfs_ioc_setxflags(
> > FS_SYNC_FL))
> > return -EOPNOTSUPP;
> > - fa.fsx_xflags = xfs_merge_ioc_xflags(flags, xfs_ip2xflags(ip));
> > + xfs_fill_fsxattr(ip, false, &fa);
>
> While reviewing this patch, it looks like xfs_fill_fsxattr comes in with a
> different set? Not sure if you meant to stack them that way. I may come
> back to this patch later if there is a dependency. Or maybe it might make
> sense to move this patch into the set it depends on?
This series depends on the two that were posted immediately before it,
though I admit the cover letters don't really make that explicit.
--D
> Allison
>
> > + fa.fsx_xflags = xfs_merge_ioc_xflags(flags, fa.fsx_xflags);
> > error = mnt_want_write_file(filp);
> > if (error)
> > return error;
> > -
> > - error = xfs_ioctl_setattr_drain_writes(ip, &fa, &join_flags);
> > - if (error) {
> > - xfs_iunlock(ip, join_flags);
> > - goto out_drop_write;
> > - }
> > -
> > - /*
> > - * 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.
> > - */
> > - error = xfs_ioctl_setattr_dax_invalidate(ip, &fa, &join_flags);
> > - if (error) {
> > - xfs_iunlock(ip, join_flags);
> > - goto out_drop_write;
> > - }
> > -
> > - tp = xfs_ioctl_setattr_get_trans(ip, join_flags);
> > - if (IS_ERR(tp)) {
> > - error = PTR_ERR(tp);
> > - goto out_drop_write;
> > - }
> > -
> > - xfs_fill_fsxattr(ip, false, &old_fa);
> > - error = vfs_ioc_fssetxattr_check(VFS_I(ip), &old_fa, &fa);
> > - if (error) {
> > - xfs_trans_cancel(tp);
> > - goto out_drop_write;
> > - }
> > -
> > - error = xfs_ioctl_setattr_xflags(tp, ip, &fa);
> > - if (error) {
> > - xfs_trans_cancel(tp);
> > - goto out_drop_write;
> > - }
> > -
> > - error = xfs_trans_commit(tp);
> > -out_drop_write:
> > + error = xfs_ioctl_setattr(ip, &fa);
> > mnt_drop_write_file(filp);
> > return error;
> > }
> >
