| Message ID | 1549609355-11053-1-git-send-email-anand.jain@oracle.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Misc props.c cleanups | expand |
On 8.02.19 г. 9:02 ч., Anand Jain wrote: > While adding the readmirror property found few cleanup things which > can be fixed. As these aren't part of upcoming readmirror property > I am sending these separately. Except for 1/5 rest fixes are in > props.c. 1/5 is in volume.c. > > Anand Jain (5): > btrfs: fix comment its device list mutex not volume lock > btrfs: kill __btrfs_set_prop() > btrfs: reorg functions to drop forward declaration > btrfs: drop redundant forward declaration in props.c > btrfs: trivial, fix c coding style > > fs/btrfs/ioctl.c | 10 +-- > fs/btrfs/props.c | 201 ++++++++++++++++++++++++----------------------------- > fs/btrfs/props.h | 6 +- > fs/btrfs/volumes.c | 2 +- > fs/btrfs/xattr.c | 2 +- > 5 files changed, 100 insertions(+), 121 deletions(-) > Allow me to suggest to introduce another patch with the following hunk: diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c index ea78c3d6dcfc..37e9af2a9f8c 100644 --- a/fs/btrfs/xattr.c +++ b/fs/btrfs/xattr.c @@ -229,12 +229,11 @@ int btrfs_setxattr(struct btrfs_trans_handle *trans, if (btrfs_root_readonly(root)) return -EROFS; - if (trans) - return do_setxattr(trans, inode, name, value, size, flags); - - trans = btrfs_start_transaction(root, 2); - if (IS_ERR(trans)) - return PTR_ERR(trans); + if (!trans) { + trans = btrfs_start_transaction(root, 2); + if (IS_ERR(trans)) + return PTR_ERR(trans); + } ret = do_setxattr(trans, inode, name, value, size, flags); if (ret) This just inverts the logic for !trans and makes the code a bit more linear and (IMO) easier to follow.
On 2/8/19 3:15 PM, Nikolay Borisov wrote: > > > On 8.02.19 г. 9:02 ч., Anand Jain wrote: >> While adding the readmirror property found few cleanup things which >> can be fixed. As these aren't part of upcoming readmirror property >> I am sending these separately. Except for 1/5 rest fixes are in >> props.c. 1/5 is in volume.c. >> >> Anand Jain (5): >> btrfs: fix comment its device list mutex not volume lock >> btrfs: kill __btrfs_set_prop() >> btrfs: reorg functions to drop forward declaration >> btrfs: drop redundant forward declaration in props.c >> btrfs: trivial, fix c coding style >> >> fs/btrfs/ioctl.c | 10 +-- >> fs/btrfs/props.c | 201 ++++++++++++++++++++++++----------------------------- >> fs/btrfs/props.h | 6 +- >> fs/btrfs/volumes.c | 2 +- >> fs/btrfs/xattr.c | 2 +- >> 5 files changed, 100 insertions(+), 121 deletions(-) >> > > Allow me to suggest to introduce another patch with the following hunk: > > diff --git a/fs/btrfs/xattr.c b/fs/btrfs/xattr.c > index ea78c3d6dcfc..37e9af2a9f8c 100644 > --- a/fs/btrfs/xattr.c > +++ b/fs/btrfs/xattr.c > @@ -229,12 +229,11 @@ int btrfs_setxattr(struct btrfs_trans_handle *trans, > if (btrfs_root_readonly(root)) > return -EROFS; > > - if (trans) > - return do_setxattr(trans, inode, name, value, size, flags); > - > - trans = btrfs_start_transaction(root, 2); > - if (IS_ERR(trans)) > - return PTR_ERR(trans); > + if (!trans) { > + trans = btrfs_start_transaction(root, 2); > + if (IS_ERR(trans)) > + return PTR_ERR(trans); > + } > > ret = do_setxattr(trans, inode, name, value, size, flags); > if (ret) > > > This just inverts the logic for !trans and makes the code a bit more > linear and (IMO) easier to follow. > Thanks for the comments. But we still have to break the linear flow for the case of trans != NULL. Where we don't do the inode_inc_iversion(inode); and the rest part. -Anand