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@xxxxxxxxx wrote:
> > From: Ira Weiny <ira.weiny@xxxxxxxxx>
[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
