* [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop
@ 2019-04-20 11:48 Anand Jain
2019-04-20 11:48 ` [PATCH 01/10] btrfs: refactor btrfs_set_props to validate externally Anand Jain
` (10 more replies)
0 siblings, 11 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
A set of pathes to clean up btrfs_ioctl_setflags(), btrfs_set_prop_trans()
and btrfs_set_prop(). So as to avoidi duplicate code in
btrfs_ioctl_setflags() and killing of btrfs_set_prop_trans().
Anand Jain (10):
btrfs: refactor btrfs_set_props to validate externally
btrfs: export btrfs_set_prop
btrfs: start transaction in btrfs_ioctl_setflags()
btrfs: drop useless inode i_flags copy and restore
btrfs: make a local copy of btrfs_inode flags before update
btrfs: drop old_fsflags in btrfs_ioctl_setflags
btrfs: drop local copy of inode i_mode
btrfs: start transaction in xattr_handler_set_prop
btrfs: delete unused function btrfs_set_prop_trans
btrfs: btrfs_set_prop trans is never null
fs/btrfs/ioctl.c | 111 ++++++++++++++++++++++-------------------------
fs/btrfs/props.c | 56 ++++++++++--------------
fs/btrfs/props.h | 6 ++-
fs/btrfs/xattr.c | 26 ++++++++++-
4 files changed, 105 insertions(+), 94 deletions(-)
--
2.20.1 (Apple Git-117)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 01/10] btrfs: refactor btrfs_set_props to validate externally
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 02/10] btrfs: export btrfs_set_prop Anand Jain
` (9 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
In preparation to merge multiple transactions when setting the
compression flags, split btrfs_set_props() validation part
outside of it.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 3 +++
fs/btrfs/props.c | 23 +++++++++++++++++------
fs/btrfs/props.h | 1 +
fs/btrfs/xattr.c | 5 +++++
4 files changed, 26 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 8c9a908d3acc..63e6e2f5b659 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -284,6 +284,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
binode->flags &= ~BTRFS_INODE_COMPRESS;
binode->flags |= BTRFS_INODE_NOCOMPRESS;
+ /* set no-compression no need to validate prop here */
ret = btrfs_set_prop_trans(inode, "btrfs.compression", NULL,
0, 0);
if (ret && ret != -ENODATA)
@@ -299,6 +300,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
binode->flags |= BTRFS_INODE_COMPRESS;
binode->flags &= ~BTRFS_INODE_NOCOMPRESS;
+ /* compress_type is already validated during mount options */
comp = btrfs_compress_type2str(fs_info->compress_type);
if (!comp || comp[0] == 0)
comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
@@ -309,6 +311,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
goto out_drop;
} else {
+ /* reset prop, no need of validate prop here */
ret = btrfs_set_prop_trans(inode, "btrfs.compression", NULL,
0, 0);
if (ret && ret != -ENODATA)
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index 44b7bf647ab3..e356dd2a0f73 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -55,6 +55,23 @@ find_prop_handler(const char *name,
return NULL;
}
+int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
+{
+ const struct prop_handler *handler;
+
+ if (strlen(name) <= XATTR_BTRFS_PREFIX_LEN)
+ return -EINVAL;
+
+ handler = find_prop_handler(name, NULL);
+ if (!handler)
+ return -EINVAL;
+
+ if (value_len == 0)
+ return 0;
+
+ return handler->validate(value, value_len);
+}
+
static int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
const char *name, const char *value, size_t value_len,
int flags)
@@ -62,9 +79,6 @@ static int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
const struct prop_handler *handler;
int ret;
- if (strlen(name) <= XATTR_BTRFS_PREFIX_LEN)
- return -EINVAL;
-
handler = find_prop_handler(name, NULL);
if (!handler)
return -EINVAL;
@@ -85,9 +99,6 @@ static int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
return ret;
}
- ret = handler->validate(value, value_len);
- if (ret)
- return ret;
if (trans)
ret = btrfs_setxattr(trans, inode, handler->xattr_name, value,
value_len, flags);
diff --git a/fs/btrfs/props.h b/fs/btrfs/props.h
index b1a6b233b774..01d2c1899bc7 100644
--- a/fs/btrfs/props.h
+++ b/fs/btrfs/props.h
@@ -12,6 +12,7 @@ void __init btrfs_props_init(void);
int btrfs_set_prop_trans(struct inode *inode, const char *name,
const char *value, size_t value_len, int flags);
+int btrfs_validate_prop(const char *name, const char *value, size_t value_len);
int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path);
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 10da873d11f5..807cc24dc885 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -373,7 +373,12 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
const char *name, const void *value,
size_t size, int flags)
{
+ int ret;
+
name = xattr_full_name(handler, name);
+ ret = btrfs_validate_prop(name, value, size);
+ if (ret)
+ return ret;
return btrfs_set_prop_trans(inode, name, value, size, flags);
}
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 02/10] btrfs: export btrfs_set_prop
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
2019-04-20 11:48 ` [PATCH 01/10] btrfs: refactor btrfs_set_props to validate externally Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 03/10] btrfs: start transaction in btrfs_ioctl_setflags() Anand Jain
` (8 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
Make btrfs_set_prop() a non static function, so that it can be called
from btrfs_ioctl_setflags(). We need btrfs_set_prop() instead of
btrfs_set_prop_trans() so that we can use the transaction which is
already started in the current thread.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/props.c | 6 +++---
fs/btrfs/props.h | 3 +++
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index e356dd2a0f73..aedf5a7d69c9 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -72,9 +72,9 @@ int btrfs_validate_prop(const char *name, const char *value, size_t value_len)
return handler->validate(value, value_len);
}
-static int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
- const char *name, const char *value, size_t value_len,
- int flags)
+int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
+ const char *name, const char *value, size_t value_len,
+ int flags)
{
const struct prop_handler *handler;
int ret;
diff --git a/fs/btrfs/props.h b/fs/btrfs/props.h
index 01d2c1899bc7..30b99348977d 100644
--- a/fs/btrfs/props.h
+++ b/fs/btrfs/props.h
@@ -12,6 +12,9 @@ void __init btrfs_props_init(void);
int btrfs_set_prop_trans(struct inode *inode, const char *name,
const char *value, size_t value_len, int flags);
+int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
+ const char *name, const char *value, size_t value_len,
+ int flags);
int btrfs_validate_prop(const char *name, const char *value, size_t value_len);
int btrfs_load_inode_props(struct inode *inode, struct btrfs_path *path);
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 03/10] btrfs: start transaction in btrfs_ioctl_setflags()
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
2019-04-20 11:48 ` [PATCH 01/10] btrfs: refactor btrfs_set_props to validate externally Anand Jain
2019-04-20 11:48 ` [PATCH 02/10] btrfs: export btrfs_set_prop Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 04/10] btrfs: drop useless inode i_flags copy and restore Anand Jain
` (7 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
Inode attribute can be set through the FS_IOC_SETFLAGS ioctl.
This flags also includes compression attribute for which we would
set/reset the compression extended attribute. While doing this there
is a bit of duplicate code, the following things happens twice-
start/end_transaction, inode_inc_iversion(), current_time update to
inode->i_ctime, and btrfs_update_inode(). These are updated both at
btrfs_ioctl_setflags() and btrfs_set_props() as well.
This patch merges these two duplicate codes at btrfs_ioctl_setflags().
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
RFC->v1: call btrfs_transaction_abort() at the place of error return
fs/btrfs/ioctl.c | 41 +++++++++++++++++++++--------------------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 63e6e2f5b659..fc3f75c9a210 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -192,6 +192,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
u64 old_flags;
unsigned int old_i_flags;
umode_t mode;
+ const char *comp = NULL;
if (!inode_owner_or_capable(inode))
return -EPERM;
@@ -283,14 +284,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
if (fsflags & FS_NOCOMP_FL) {
binode->flags &= ~BTRFS_INODE_COMPRESS;
binode->flags |= BTRFS_INODE_NOCOMPRESS;
-
- /* set no-compression no need to validate prop here */
- ret = btrfs_set_prop_trans(inode, "btrfs.compression", NULL,
- 0, 0);
- if (ret && ret != -ENODATA)
- goto out_drop;
} else if (fsflags & FS_COMPR_FL) {
- const char *comp;
if (IS_SWAPFILE(inode)) {
ret = -ETXTBSY;
@@ -300,36 +294,43 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
binode->flags |= BTRFS_INODE_COMPRESS;
binode->flags &= ~BTRFS_INODE_NOCOMPRESS;
- /* compress_type is already validated during mount options */
comp = btrfs_compress_type2str(fs_info->compress_type);
if (!comp || comp[0] == 0)
comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
-
- ret = btrfs_set_prop_trans(inode, "btrfs.compression", comp,
- strlen(comp), 0);
- if (ret)
- goto out_drop;
-
} else {
- /* reset prop, no need of validate prop here */
- ret = btrfs_set_prop_trans(inode, "btrfs.compression", NULL,
- 0, 0);
- if (ret && ret != -ENODATA)
- goto out_drop;
binode->flags &= ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
}
- trans = btrfs_start_transaction(root, 1);
+ trans = btrfs_start_transaction(root, 3);
if (IS_ERR(trans)) {
ret = PTR_ERR(trans);
goto out_drop;
}
+ if (comp) {
+ ret = btrfs_set_prop(trans, inode, "btrfs.compression", comp,
+ strlen(comp), 0);
+ if (ret) {
+ 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);
+ if (ret && ret != -ENODATA) {
+ btrfs_abort_transaction(trans, ret);
+ goto out_end_trans;
+ }
+ }
+
btrfs_sync_inode_flags_to_i_flags(inode);
inode_inc_iversion(inode);
inode->i_ctime = current_time(inode);
ret = btrfs_update_inode(trans, root, inode);
+ out_end_trans:
btrfs_end_transaction(trans);
out_drop:
if (ret) {
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 04/10] btrfs: drop useless inode i_flags copy and restore
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (2 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 03/10] btrfs: start transaction in btrfs_ioctl_setflags() Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 05/10] btrfs: make a local copy of btrfs_inode flags before update Anand Jain
` (6 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
The patch (btrfs: start transaction in btrfs_ioctl_setflags())
used btrfs_set_prop() instead of btrfs_set_prop_trans() by which now
the inode::i_flags update functions such as
btrfs_sync_inode_flags_to_i_flags() and btrfs_update_inode() is called
in btrfs_ioctl_setflags() instead of
btrfs_set_prop_trans()->btrfs_setxattr() as earlier. So the inode::i_flags
remains unmodified until the thread has checked all the conditions. So drop
the saved inode::i_flags in out_i_flags.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index fc3f75c9a210..1834b576d1cb 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -190,7 +190,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
unsigned int fsflags, old_fsflags;
int ret;
u64 old_flags;
- unsigned int old_i_flags;
umode_t mode;
const char *comp = NULL;
@@ -214,7 +213,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
inode_lock(inode);
old_flags = binode->flags;
- old_i_flags = inode->i_flags;
mode = inode->i_mode;
fsflags = btrfs_mask_fsflags_for_type(inode, fsflags);
@@ -335,7 +333,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
out_drop:
if (ret) {
binode->flags = old_flags;
- inode->i_flags = old_i_flags;
}
out_unlock:
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 05/10] btrfs: make a local copy of btrfs_inode flags before update
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (3 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 04/10] btrfs: drop useless inode i_flags copy and restore Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 06/10] btrfs: drop old_fsflags in btrfs_ioctl_setflags Anand Jain
` (5 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
Instead of updating the binode::flags directly, update a local copy,
and then at the point of no error-return copy it to the binode::flags.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 57 ++++++++++++++++++++++--------------------------
1 file changed, 26 insertions(+), 31 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 1834b576d1cb..b87a249642f7 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -189,9 +189,9 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
struct btrfs_trans_handle *trans;
unsigned int fsflags, old_fsflags;
int ret;
- u64 old_flags;
umode_t mode;
const char *comp = NULL;
+ u32 binode_flags = binode->flags;
if (!inode_owner_or_capable(inode))
return -EPERM;
@@ -212,7 +212,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
inode_lock(inode);
- old_flags = binode->flags;
mode = inode->i_mode;
fsflags = btrfs_mask_fsflags_for_type(inode, fsflags);
@@ -225,29 +224,29 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
}
if (fsflags & FS_SYNC_FL)
- binode->flags |= BTRFS_INODE_SYNC;
+ binode_flags |= BTRFS_INODE_SYNC;
else
- binode->flags &= ~BTRFS_INODE_SYNC;
+ binode_flags &= ~BTRFS_INODE_SYNC;
if (fsflags & FS_IMMUTABLE_FL)
- binode->flags |= BTRFS_INODE_IMMUTABLE;
+ binode_flags |= BTRFS_INODE_IMMUTABLE;
else
- binode->flags &= ~BTRFS_INODE_IMMUTABLE;
+ binode_flags &= ~BTRFS_INODE_IMMUTABLE;
if (fsflags & FS_APPEND_FL)
- binode->flags |= BTRFS_INODE_APPEND;
+ binode_flags |= BTRFS_INODE_APPEND;
else
- binode->flags &= ~BTRFS_INODE_APPEND;
+ binode_flags &= ~BTRFS_INODE_APPEND;
if (fsflags & FS_NODUMP_FL)
- binode->flags |= BTRFS_INODE_NODUMP;
+ binode_flags |= BTRFS_INODE_NODUMP;
else
- binode->flags &= ~BTRFS_INODE_NODUMP;
+ binode_flags &= ~BTRFS_INODE_NODUMP;
if (fsflags & FS_NOATIME_FL)
- binode->flags |= BTRFS_INODE_NOATIME;
+ binode_flags |= BTRFS_INODE_NOATIME;
else
- binode->flags &= ~BTRFS_INODE_NOATIME;
+ binode_flags &= ~BTRFS_INODE_NOATIME;
if (fsflags & FS_DIRSYNC_FL)
- binode->flags |= BTRFS_INODE_DIRSYNC;
+ binode_flags |= BTRFS_INODE_DIRSYNC;
else
- binode->flags &= ~BTRFS_INODE_DIRSYNC;
+ binode_flags &= ~BTRFS_INODE_DIRSYNC;
if (fsflags & FS_NOCOW_FL) {
if (S_ISREG(mode)) {
/*
@@ -256,10 +255,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
* status of the file and will not set it.
*/
if (inode->i_size == 0)
- binode->flags |= BTRFS_INODE_NODATACOW
- | BTRFS_INODE_NODATASUM;
+ binode_flags |= BTRFS_INODE_NODATACOW |
+ BTRFS_INODE_NODATASUM;
} else {
- binode->flags |= BTRFS_INODE_NODATACOW;
+ binode_flags |= BTRFS_INODE_NODATACOW;
}
} else {
/*
@@ -267,10 +266,10 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
*/
if (S_ISREG(mode)) {
if (inode->i_size == 0)
- binode->flags &= ~(BTRFS_INODE_NODATACOW
- | BTRFS_INODE_NODATASUM);
+ binode_flags &= ~(BTRFS_INODE_NODATACOW |
+ BTRFS_INODE_NODATASUM);
} else {
- binode->flags &= ~BTRFS_INODE_NODATACOW;
+ binode_flags &= ~BTRFS_INODE_NODATACOW;
}
}
@@ -280,8 +279,8 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
* things smaller.
*/
if (fsflags & FS_NOCOMP_FL) {
- binode->flags &= ~BTRFS_INODE_COMPRESS;
- binode->flags |= BTRFS_INODE_NOCOMPRESS;
+ binode_flags &= ~BTRFS_INODE_COMPRESS;
+ binode_flags |= BTRFS_INODE_NOCOMPRESS;
} else if (fsflags & FS_COMPR_FL) {
if (IS_SWAPFILE(inode)) {
@@ -289,20 +288,20 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
goto out_unlock;
}
- binode->flags |= BTRFS_INODE_COMPRESS;
- binode->flags &= ~BTRFS_INODE_NOCOMPRESS;
+ binode_flags |= BTRFS_INODE_COMPRESS;
+ binode_flags &= ~BTRFS_INODE_NOCOMPRESS;
comp = btrfs_compress_type2str(fs_info->compress_type);
if (!comp || comp[0] == 0)
comp = btrfs_compress_type2str(BTRFS_COMPRESS_ZLIB);
} else {
- binode->flags &= ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
+ binode_flags &= ~(BTRFS_INODE_COMPRESS | BTRFS_INODE_NOCOMPRESS);
}
trans = btrfs_start_transaction(root, 3);
if (IS_ERR(trans)) {
ret = PTR_ERR(trans);
- goto out_drop;
+ goto out_unlock;
}
if (comp) {
@@ -323,6 +322,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
}
}
+ binode->flags = binode_flags;
btrfs_sync_inode_flags_to_i_flags(inode);
inode_inc_iversion(inode);
inode->i_ctime = current_time(inode);
@@ -330,11 +330,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
out_end_trans:
btrfs_end_transaction(trans);
- out_drop:
- if (ret) {
- binode->flags = old_flags;
- }
-
out_unlock:
inode_unlock(inode);
mnt_drop_write_file(file);
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 06/10] btrfs: drop old_fsflags in btrfs_ioctl_setflags
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (4 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 05/10] btrfs: make a local copy of btrfs_inode flags before update Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 07/10] btrfs: drop local copy of inode i_mode Anand Jain
` (4 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
btrfs_inode_flags_to_fsflags() is copied into %old_fsflags and used only
once. Instead used it directly.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index b87a249642f7..e83412d7363a 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -187,7 +187,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
struct btrfs_inode *binode = BTRFS_I(inode);
struct btrfs_root *root = binode->root;
struct btrfs_trans_handle *trans;
- unsigned int fsflags, old_fsflags;
+ unsigned int fsflags;
int ret;
umode_t mode;
const char *comp = NULL;
@@ -215,8 +215,8 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
mode = inode->i_mode;
fsflags = btrfs_mask_fsflags_for_type(inode, fsflags);
- old_fsflags = btrfs_inode_flags_to_fsflags(binode->flags);
- if ((fsflags ^ old_fsflags) & (FS_APPEND_FL | FS_IMMUTABLE_FL)) {
+ if ((fsflags ^ btrfs_inode_flags_to_fsflags(binode->flags)) &
+ (FS_APPEND_FL | FS_IMMUTABLE_FL)) {
if (!capable(CAP_LINUX_IMMUTABLE)) {
ret = -EPERM;
goto out_unlock;
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 07/10] btrfs: drop local copy of inode i_mode
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (5 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 06/10] btrfs: drop old_fsflags in btrfs_ioctl_setflags Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 08/10] btrfs: start transaction in xattr_handler_set_prop Anand Jain
` (3 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
There isn't real use of making struct inode::i_mode a local copy, it saves
a dereference one time, not much. Just use it directly.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/ioctl.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index e83412d7363a..142fcdaf6a93 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -189,7 +189,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
struct btrfs_trans_handle *trans;
unsigned int fsflags;
int ret;
- umode_t mode;
const char *comp = NULL;
u32 binode_flags = binode->flags;
@@ -212,8 +211,6 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
inode_lock(inode);
- mode = inode->i_mode;
-
fsflags = btrfs_mask_fsflags_for_type(inode, fsflags);
if ((fsflags ^ btrfs_inode_flags_to_fsflags(binode->flags)) &
(FS_APPEND_FL | FS_IMMUTABLE_FL)) {
@@ -248,7 +245,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
else
binode_flags &= ~BTRFS_INODE_DIRSYNC;
if (fsflags & FS_NOCOW_FL) {
- if (S_ISREG(mode)) {
+ if (S_ISREG(inode->i_mode)) {
/*
* It's safe to turn csums off here, no extents exist.
* Otherwise we want the flag to reflect the real COW
@@ -264,7 +261,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg)
/*
* Revert back under same assumptions as above
*/
- if (S_ISREG(mode)) {
+ if (S_ISREG(inode->i_mode)) {
if (inode->i_size == 0)
binode_flags &= ~(BTRFS_INODE_NODATACOW |
BTRFS_INODE_NODATASUM);
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 08/10] btrfs: start transaction in xattr_handler_set_prop
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (6 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 07/10] btrfs: drop local copy of inode i_mode Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:48 ` [PATCH 09/10] btrfs: delete unused function btrfs_set_prop_trans Anand Jain
` (2 subsequent siblings)
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
btrfs specific extended attributes on the inode are set using
btrfs_xattr_handler_set_prop(), and the required transaction for this
update is started by btrfs_setxattr(). For better visibility of the
transaction start and end, do this in btrfs_xattr_handler_set_prop().
For which this patch copied code of btrfs_setxattr() as it is in the
original, which needs proper error handling.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/xattr.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c
index 807cc24dc885..46ad7b5cf209 100644
--- a/fs/btrfs/xattr.c
+++ b/fs/btrfs/xattr.c
@@ -374,12 +374,31 @@ static int btrfs_xattr_handler_set_prop(const struct xattr_handler *handler,
size_t size, int flags)
{
int ret;
+ struct btrfs_trans_handle *trans;
+ struct btrfs_root *root = BTRFS_I(inode)->root;
name = xattr_full_name(handler, name);
ret = btrfs_validate_prop(name, value, size);
if (ret)
return ret;
- return btrfs_set_prop_trans(inode, name, value, size, flags);
+
+ trans = btrfs_start_transaction(root, 2);
+ if (IS_ERR(trans))
+ return PTR_ERR(trans);
+
+ ret = btrfs_set_prop(trans, inode, name, value, size, flags);
+ 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);
+ }
+
+ btrfs_end_transaction(trans);
+
+ return ret;
}
static const struct xattr_handler btrfs_security_xattr_handler = {
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 09/10] btrfs: delete unused function btrfs_set_prop_trans
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (7 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 08/10] btrfs: start transaction in xattr_handler_set_prop Anand Jain
@ 2019-04-20 11:48 ` Anand Jain
2019-04-20 11:49 ` [PATCH 10/10] btrfs: btrfs_set_prop trans is never null Anand Jain
2019-04-24 15:14 ` [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop David Sterba
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:48 UTC (permalink / raw)
To: linux-btrfs
The last consumer of btrfs_set_prop_trans() was taken away by the patch
btrfs: start transaction in xattr_handler_set_prop
so now this function can be deleted.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/props.c | 6 ------
fs/btrfs/props.h | 2 --
2 files changed, 8 deletions(-)
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index aedf5a7d69c9..c41ed2a6347e 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -124,12 +124,6 @@ int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
return 0;
}
-int btrfs_set_prop_trans(struct inode *inode, const char *name,
- const char *value, size_t value_len, int flags)
-{
- return btrfs_set_prop(NULL, inode, name, value, value_len, flags);
-}
-
static int iterate_object_props(struct btrfs_root *root,
struct btrfs_path *path,
u64 objectid,
diff --git a/fs/btrfs/props.h b/fs/btrfs/props.h
index 30b99348977d..40b2c65b518c 100644
--- a/fs/btrfs/props.h
+++ b/fs/btrfs/props.h
@@ -10,8 +10,6 @@
void __init btrfs_props_init(void);
-int btrfs_set_prop_trans(struct inode *inode, const char *name,
- const char *value, size_t value_len, int flags);
int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
const char *name, const char *value, size_t value_len,
int flags);
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 10/10] btrfs: btrfs_set_prop trans is never null
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (8 preceding siblings ...)
2019-04-20 11:48 ` [PATCH 09/10] btrfs: delete unused function btrfs_set_prop_trans Anand Jain
@ 2019-04-20 11:49 ` Anand Jain
2019-04-24 15:14 ` [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop David Sterba
10 siblings, 0 replies; 14+ messages in thread
From: Anand Jain @ 2019-04-20 11:49 UTC (permalink / raw)
To: linux-btrfs
Since now the trans argument is never null we don't have to check if
its null. So delete it.
Signed-off-by: Anand Jain <anand.jain@oracle.com>
---
fs/btrfs/props.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/fs/btrfs/props.c b/fs/btrfs/props.c
index c41ed2a6347e..ca2716917e37 100644
--- a/fs/btrfs/props.c
+++ b/fs/btrfs/props.c
@@ -84,12 +84,8 @@ int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
return -EINVAL;
if (value_len == 0) {
- if (trans)
- ret = btrfs_setxattr(trans, inode, handler->xattr_name,
- NULL, 0, flags);
- else
- ret = btrfs_setxattr_trans(inode, handler->xattr_name,
- NULL, 0, flags);
+ ret = btrfs_setxattr(trans, inode, handler->xattr_name,
+ NULL, 0, flags);
if (ret)
return ret;
@@ -99,23 +95,14 @@ int btrfs_set_prop(struct btrfs_trans_handle *trans, struct inode *inode,
return ret;
}
- if (trans)
- ret = btrfs_setxattr(trans, inode, handler->xattr_name, value,
- value_len, flags);
- else
- ret = btrfs_setxattr_trans(inode, handler->xattr_name, value,
- value_len, flags);
-
+ ret = btrfs_setxattr(trans, inode, handler->xattr_name, value,
+ value_len, flags);
if (ret)
return ret;
ret = handler->apply(inode, value, value_len);
if (ret) {
- if (trans)
- btrfs_setxattr(trans, inode, handler->xattr_name, NULL,
- 0, flags);
- else
- btrfs_setxattr_trans(inode, handler->xattr_name, NULL,
- 0, flags);
+ btrfs_setxattr(trans, inode, handler->xattr_name, NULL,
+ 0, flags);
return ret;
}
--
2.20.1 (Apple Git-117)
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
` (9 preceding siblings ...)
2019-04-20 11:49 ` [PATCH 10/10] btrfs: btrfs_set_prop trans is never null Anand Jain
@ 2019-04-24 15:14 ` David Sterba
2019-04-26 6:20 ` Anand Jain
10 siblings, 1 reply; 14+ messages in thread
From: David Sterba @ 2019-04-24 15:14 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Sat, Apr 20, 2019 at 07:48:50PM +0800, Anand Jain wrote:
> A set of pathes to clean up btrfs_ioctl_setflags(), btrfs_set_prop_trans()
> and btrfs_set_prop(). So as to avoidi duplicate code in
> btrfs_ioctl_setflags() and killing of btrfs_set_prop_trans().
>
> Anand Jain (10):
> btrfs: refactor btrfs_set_props to validate externally
> btrfs: export btrfs_set_prop
> btrfs: start transaction in btrfs_ioctl_setflags()
> btrfs: drop useless inode i_flags copy and restore
> btrfs: make a local copy of btrfs_inode flags before update
> btrfs: drop old_fsflags in btrfs_ioctl_setflags
> btrfs: drop local copy of inode i_mode
> btrfs: start transaction in xattr_handler_set_prop
> btrfs: delete unused function btrfs_set_prop_trans
> btrfs: btrfs_set_prop trans is never null
Looks good to me, I'll add it to misc-next as it's only a cleanup
series, now that we're getting close to 5.2 code freeze.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop
2019-04-24 15:14 ` [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop David Sterba
@ 2019-04-26 6:20 ` Anand Jain
2019-04-26 11:49 ` David Sterba
0 siblings, 1 reply; 14+ messages in thread
From: Anand Jain @ 2019-04-26 6:20 UTC (permalink / raw)
To: dsterba; +Cc: linux-btrfs
On 24/4/19 11:14 PM, David Sterba wrote:
> On Sat, Apr 20, 2019 at 07:48:50PM +0800, Anand Jain wrote:
>> A set of pathes to clean up btrfs_ioctl_setflags(), btrfs_set_prop_trans()
>> and btrfs_set_prop(). So as to avoidi duplicate code in
>> btrfs_ioctl_setflags() and killing of btrfs_set_prop_trans().
>>
>> Anand Jain (10):
>> btrfs: refactor btrfs_set_props to validate externally
>> btrfs: export btrfs_set_prop
>> btrfs: start transaction in btrfs_ioctl_setflags()
>> btrfs: drop useless inode i_flags copy and restore
>> btrfs: make a local copy of btrfs_inode flags before update
>> btrfs: drop old_fsflags in btrfs_ioctl_setflags
>> btrfs: drop local copy of inode i_mode
>> btrfs: start transaction in xattr_handler_set_prop
>> btrfs: delete unused function btrfs_set_prop_trans
>> btrfs: btrfs_set_prop trans is never null
>
> Looks good to me, I'll add it to misc-next as it's only a cleanup
> series, now that we're getting close to 5.2 code freeze.
>
Thanks.
I thought I will take a look, looks like its not yet pushed?
Which is the most updated? repo gitlab/github?
-Anand
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop
2019-04-26 6:20 ` Anand Jain
@ 2019-04-26 11:49 ` David Sterba
0 siblings, 0 replies; 14+ messages in thread
From: David Sterba @ 2019-04-26 11:49 UTC (permalink / raw)
To: Anand Jain; +Cc: linux-btrfs
On Fri, Apr 26, 2019 at 02:20:08PM +0800, Anand Jain wrote:
> On 24/4/19 11:14 PM, David Sterba wrote:
> > On Sat, Apr 20, 2019 at 07:48:50PM +0800, Anand Jain wrote:
> >> A set of pathes to clean up btrfs_ioctl_setflags(), btrfs_set_prop_trans()
> >> and btrfs_set_prop(). So as to avoidi duplicate code in
> >> btrfs_ioctl_setflags() and killing of btrfs_set_prop_trans().
> >>
> >> Anand Jain (10):
> >> btrfs: refactor btrfs_set_props to validate externally
> >> btrfs: export btrfs_set_prop
> >> btrfs: start transaction in btrfs_ioctl_setflags()
> >> btrfs: drop useless inode i_flags copy and restore
> >> btrfs: make a local copy of btrfs_inode flags before update
> >> btrfs: drop old_fsflags in btrfs_ioctl_setflags
> >> btrfs: drop local copy of inode i_mode
> >> btrfs: start transaction in xattr_handler_set_prop
> >> btrfs: delete unused function btrfs_set_prop_trans
> >> btrfs: btrfs_set_prop trans is never null
> >
> > Looks good to me, I'll add it to misc-next as it's only a cleanup
> > series, now that we're getting close to 5.2 code freeze.
> >
> Thanks.
> I thought I will take a look, looks like its not yet pushed?
> Which is the most updated? repo gitlab/github?
It's been in for-next until yesterday and I've merged it to misc-next
after that so it'll be out today.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-26 11:48 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-20 11:48 [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop Anand Jain
2019-04-20 11:48 ` [PATCH 01/10] btrfs: refactor btrfs_set_props to validate externally Anand Jain
2019-04-20 11:48 ` [PATCH 02/10] btrfs: export btrfs_set_prop Anand Jain
2019-04-20 11:48 ` [PATCH 03/10] btrfs: start transaction in btrfs_ioctl_setflags() Anand Jain
2019-04-20 11:48 ` [PATCH 04/10] btrfs: drop useless inode i_flags copy and restore Anand Jain
2019-04-20 11:48 ` [PATCH 05/10] btrfs: make a local copy of btrfs_inode flags before update Anand Jain
2019-04-20 11:48 ` [PATCH 06/10] btrfs: drop old_fsflags in btrfs_ioctl_setflags Anand Jain
2019-04-20 11:48 ` [PATCH 07/10] btrfs: drop local copy of inode i_mode Anand Jain
2019-04-20 11:48 ` [PATCH 08/10] btrfs: start transaction in xattr_handler_set_prop Anand Jain
2019-04-20 11:48 ` [PATCH 09/10] btrfs: delete unused function btrfs_set_prop_trans Anand Jain
2019-04-20 11:49 ` [PATCH 10/10] btrfs: btrfs_set_prop trans is never null Anand Jain
2019-04-24 15:14 ` [PATCH 00/10] cleanup btrfs_ioctl_setflags and btrfs_set_prop David Sterba
2019-04-26 6:20 ` Anand Jain
2019-04-26 11:49 ` David Sterba
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).