lkml.org 
[lkml]   [2020]   [Jan]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC PATCH V2 08/12] fs/xfs: Add lock/unlock mode to xfs
On Mon, Jan 13, 2020 at 02:19:57PM -0800, Darrick J. Wong wrote:
> On Fri, Jan 10, 2020 at 11:29:38AM -0800, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>

[snip]

> >
> > diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> > index 401da197f012..e8fd95b75e5b 100644
> > --- a/fs/xfs/xfs_inode.c
> > +++ b/fs/xfs/xfs_inode.c
> > @@ -142,12 +142,12 @@ xfs_ilock_attr_map_shared(
> > *
> > * Basic locking order:
> > *
> > - * i_rwsem -> i_mmap_lock -> page_lock -> i_ilock
> > + * i_rwsem -> i_dax_sem -> i_mmap_lock -> page_lock -> i_ilock
>
> Mmmmmm, more locks. Can we skip the extra lock if CONFIG_FSDAX=n or if
> the filesystem devices don't support DAX at all?

I'll look into it.

>
> Also, I don't think we're actually following the i_rwsem -> i_daxsem
> order in fallocate, and possibly elsewhere too?

I'll have to verify. It took a lot of iterations to get the order working so
I'm not going to claim perfection.

>
> Does the vfs have to take the i_dax_sem to do remapping things like
> reflink? (Pretend that reflink and dax are compatible for the moment)

Honestly I can't say for sure. For this series I was careful to exclude
reflink from the locking requirement.

[snip]

> >
> > #define XFS_LOCK_FLAGS \
> > { XFS_IOLOCK_EXCL, "IOLOCK_EXCL" }, \
> > @@ -289,7 +295,9 @@ static inline void xfs_ifunlock(struct xfs_inode *ip)
> > { XFS_ILOCK_EXCL, "ILOCK_EXCL" }, \
> > { XFS_ILOCK_SHARED, "ILOCK_SHARED" }, \
> > { XFS_MMAPLOCK_EXCL, "MMAPLOCK_EXCL" }, \
> > - { XFS_MMAPLOCK_SHARED, "MMAPLOCK_SHARED" }
> > + { XFS_MMAPLOCK_SHARED, "MMAPLOCK_SHARED" }, \
> > + { XFS_DAX_EXCL, "DAX_EXCL" }, \
>
> Whitespace between the comma & string.

Fixed.

[snip]

> > +
> > static const struct inode_operations xfs_dir_inode_operations = {
> > .create = xfs_vn_create,
> > .lookup = xfs_vn_lookup,
> > @@ -1372,7 +1394,7 @@ xfs_setup_iops(
> >
> > switch (inode->i_mode & S_IFMT) {
> > case S_IFREG:
> > - inode->i_op = &xfs_inode_operations;
> > + inode->i_op = &xfs_reg_inode_operations;
>
> xfs_file_inode_operations?

Sounds better. Fixed.

Ira

\
 
 \ /
  Last update: 2020-01-14 01:37    [W:0.118 / U:0.492 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site