Re: [PATCH 4/8] btrfs: Introduce btrfs_inode_lock()/unlock()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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. 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.

-- 
Goldwyn



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux