Message ID | 20190612141411.25339-1-fdmanana@kernel.org (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [for,5.2] Btrfs: fix failure to persist compression property xattr deletion on fsync | expand |
On Wed, Jun 12, 2019 at 03:14:11PM +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > After the recent series of cleanups in the properties and xattrs modules > that landed in the 5.2 merge window, we ended up with a regression where > after deleting the compression xattr property through the setflags ioctl, > we don't set the BTRFS_INODE_COPY_EVERYTHING flag in the inode anymore. > As a consequence, if the inode was fsync'ed when it had the compression > property set, after deleting the compression property through the setflags > ioctl and fsync'ing again the inode, the log will still contain the > compression xattr, because the inode did not had that bit set, which > made the fsync not delete all xattrs from the log and copy all xattrs > from the subvolume tree to the log tree. > > This regression happens due to the fact that that series of cleanups > made btrfs_set_prop() call the old function do_setxattr() (which is now > named btrfs_setxattr()), and not the old version of btrfs_setxattr(), > which is now called btrfs_setxattr_trans(). > > Fix this by setting the BTRFS_INODE_COPY_EVERYTHING bit in the current > btrfs_setxattr() function and remove it from everywhere else, including > its setup at btrfs_ioctl_setflags(). This is cleaner, avoids similar > regressions in the future, and centralizes the setup of the bit. After > all, the need to setup this bit should only be in the xattrs module, > since it is an implementation of xattrs. > > Fixes: 04e6863b19c722 ("btrfs: split btrfs_setxattr calls regarding transaction") > Signed-off-by: Filipe Manana <fdmanana@suse.com> Queued for 5.2, thanks.
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 6dafa857bbb9..2a1be0d1a698 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -312,8 +312,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) btrfs_abort_transaction(trans, ret); goto out_end_trans; } - set_bit(BTRFS_INODE_COPY_EVERYTHING, - &BTRFS_I(inode)->runtime_flags); } else { ret = btrfs_set_prop(trans, inode, "btrfs.compression", NULL, 0, 0); diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c index 78b6ba2029e8..95d9aebff2c4 100644 --- a/fs/btrfs/xattr.c +++ b/fs/btrfs/xattr.c @@ -213,6 +213,9 @@ int btrfs_setxattr(struct btrfs_trans_handle *trans, struct inode *inode, } out: btrfs_free_path(path); + if (!ret) + set_bit(BTRFS_INODE_COPY_EVERYTHING, + &BTRFS_I(inode)->runtime_flags); return ret; } @@ -236,7 +239,6 @@ int btrfs_setxattr_trans(struct inode *inode, const char *name, inode_inc_iversion(inode); inode->i_ctime = current_time(inode); - set_bit(BTRFS_INODE_COPY_EVERYTHING, &BTRFS_I(inode)->runtime_flags); ret = btrfs_update_inode(trans, root, inode); BUG_ON(ret); out: @@ -388,8 +390,6 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler, if (!ret) { inode_inc_iversion(inode); inode->i_ctime = current_time(inode); - set_bit(BTRFS_INODE_COPY_EVERYTHING, - &BTRFS_I(inode)->runtime_flags); ret = btrfs_update_inode(trans, root, inode); BUG_ON(ret); }