On 6/25/20 7:34 PM, Goldwyn Rodrigues wrote: > On 18:19 24/06, David Sterba wrote: >> On Mon, Jun 22, 2020 at 11:20:13AM -0500, Goldwyn Rodrigues wrote: >>> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>> >>> btrfs_inode_lock/unlock() acquires the inode->i_rwsem depending on the >>> flags passed. ilock_flags determines the type of lock to be taken: >>> >>> BTRFS_ILOCK_SHARED - for shared locks, for possible parallel DIO >>> BTRFS_ILOCK_TRY - for the RWF_NOWAIT sequence >>> >>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>> --- >>> fs/btrfs/ctree.h | 8 ++++++++ >>> fs/btrfs/file.c | 51 +++++++++++++++++++++++++++++++++++++++--------- >>> 2 files changed, 50 insertions(+), 9 deletions(-) >>> >>> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h >>> index 161533040978..346fea668ca0 100644 >>> --- a/fs/btrfs/ctree.h >>> +++ b/fs/btrfs/ctree.h >>> @@ -2953,6 +2953,14 @@ void btrfs_update_ioctl_balance_args(struct btrfs_fs_info *fs_info, >>> struct btrfs_ioctl_balance_args *bargs); >>> >>> /* file.c */ >>> + >>> +/* ilock flags definition */ >>> +#define BTRFS_ILOCK_SHARED 0x1 >>> +#define BTRFS_ILOCK_TRY 0x2 >> >> Please use enums and add them to a new file inode.h. > > These are bitwise flags. I suspect that in that case it's more common to do: #define BTRFS_ILOCK_SHARED (1 << 0) #define BTRFS_ILOCK_TRY (1 << 1) > Wouldn't it be ugly if you have to set the flag > with: > > flags |= (1 << BTRFS_ILOCK_TRY); > > rather than > > flags |= BTRFS_ILOCK_TRY; > >> >>> +int btrfs_inode_lock(struct inode *inode, int ilock_flags); >>> +void btrfs_inode_unlock(struct inode *inode, int ilock_flags); >>> + >>> int __init btrfs_auto_defrag_init(void); >>> void __cold btrfs_auto_defrag_exit(void); >>> int btrfs_add_inode_defrag(struct btrfs_trans_handle *trans, >>> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c >>> index ba7c3b2cf1c5..1a9a0a9e4b3d 100644 >>> --- a/fs/btrfs/file.c >>> +++ b/fs/btrfs/file.c >>> @@ -70,6 +70,37 @@ static int __compare_inode_defrag(struct inode_defrag *defrag1, >>> return 0; >>> } >>> >>> +int btrfs_inode_lock(struct inode *inode, int ilock_flags) >>> +void btrfs_inode_unlock(struct inode *inode, int ilock_flags) >> >> And the helpers should be in inode.c, not file.c > > Yes, inode.c is more appropriate. Perhaps I should change other calls to > inode_lock(inode) to btrfs_inode_lock(inode, 0) for uniformity. > Hans
