Skip to content

Commit

Permalink
xfs: remove i_iolock and use i_rwsem in the VFS inode instead
Browse files Browse the repository at this point in the history 
This patch drops the XFS-own i_iolock and uses the VFS i_rwsem which
recently replaced i_mutex instead.  This means we only have to take
one lock instead of two in many fast path operations, and we can
also shrink the xfs_inode structure.  Thanks to the xfs_ilock family
there is very little churn, the only thing of note is that we need
to switch to use the lock_two_directory helper for taking the i_rwsem
on two inodes in a few places to make sure our lock order matches
the one used in the VFS.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Tested-by: Jens Axboe <axboe@fb.com>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Dave Chinner <david@fromorbit.com>
  • Loading branch information
@dchinner
Christoph Hellwig authored and dchinner committed Nov 30, 2016
1 parent f831948 commit 6552321
Show file tree
Hide file tree
Showing 14 changed files with 86 additions and 159 deletions.
7 changes: 2 additions & 5 deletions fs/xfs/xfs_aops.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1585,20 +1585,17 @@ xfs_vm_bmap(
struct xfs_inode *ip = XFS_I(inode);

trace_xfs_vm_bmap(XFS_I(inode));
xfs_ilock(ip, XFS_IOLOCK_SHARED);

/*
* The swap code (ab-)uses ->bmap to get a block mapping and then
* bypasseѕ the file system for actual I/O. We really can't allow
* that on reflinks inodes, so we have to skip out here. And yes,
* 0 is the magic code for a bmap error..
*/
if (xfs_is_reflink_inode(ip)) {
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
if (xfs_is_reflink_inode(ip))
return 0;
}

filemap_write_and_wait(mapping);
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
return generic_block_bmap(mapping, block, xfs_get_blocks);
}

Expand Down
12 changes: 5 additions & 7 deletions fs/xfs/xfs_bmap_util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1935,8 +1935,8 @@ xfs_swap_extents(
* page cache safely. Once we have done this we can take the ilocks and
* do the rest of the checks.
*/
lock_flags = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL);
lock_two_nondirectories(VFS_I(ip), VFS_I(tip));
lock_flags = XFS_MMAPLOCK_EXCL;
xfs_lock_two_inodes(ip, tip, XFS_MMAPLOCK_EXCL);

/* Verify that both files have the same format */
Expand Down Expand Up @@ -2076,15 +2076,13 @@ xfs_swap_extents(
trace_xfs_swap_extent_after(ip, 0);
trace_xfs_swap_extent_after(tip, 1);

out_unlock:
xfs_iunlock(ip, lock_flags);
xfs_iunlock(tip, lock_flags);
unlock_two_nondirectories(VFS_I(ip), VFS_I(tip));
return error;

out_trans_cancel:
xfs_trans_cancel(tp);

out_unlock:
xfs_iunlock(ip, lock_flags);
xfs_iunlock(tip, lock_flags);
return error;
goto out_unlock;
}
2 changes: 0 additions & 2 deletions fs/xfs/xfs_dir2_readdir.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,6 @@ xfs_readdir(
args.dp = dp;
args.geo = dp->i_mount->m_dir_geo;

xfs_ilock(dp, XFS_IOLOCK_SHARED);
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
rval = xfs_dir2_sf_getdents(&args, ctx);
else if ((rval = xfs_dir2_isblock(&args, &v)))
Expand All @@ -686,7 +685,6 @@ xfs_readdir(
rval = xfs_dir2_block_getdents(&args, ctx);
else
rval = xfs_dir2_leaf_getdents(&args, ctx, bufsize);
xfs_iunlock(dp, XFS_IOLOCK_SHARED);

return rval;
}
79 changes: 23 additions & 56 deletions fs/xfs/xfs_file.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,40 +47,6 @@

static const struct vm_operations_struct xfs_file_vm_ops;

/*
* Locking primitives for read and write IO paths to ensure we consistently use
* and order the inode->i_mutex, ip->i_lock and ip->i_iolock.
*/
static inline void
xfs_rw_ilock(
struct xfs_inode *ip,
int type)
{
if (type & XFS_IOLOCK_EXCL)
inode_lock(VFS_I(ip));
xfs_ilock(ip, type);
}

static inline void
xfs_rw_iunlock(
struct xfs_inode *ip,
int type)
{
xfs_iunlock(ip, type);
if (type & XFS_IOLOCK_EXCL)
inode_unlock(VFS_I(ip));
}

static inline void
xfs_rw_ilock_demote(
struct xfs_inode *ip,
int type)
{
xfs_ilock_demote(ip, type);
if (type & XFS_IOLOCK_EXCL)
inode_unlock(VFS_I(ip));
}

/*
* Clear the specified ranges to zero through either the pagecache or DAX.
* Holes and unwritten extents will be left as-is as they already are zeroed.
Expand Down Expand Up @@ -273,7 +239,7 @@ xfs_file_dio_aio_read(

file_accessed(iocb->ki_filp);

xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
xfs_ilock(ip, XFS_IOLOCK_SHARED);
if (mapping->nrpages) {
ret = filemap_write_and_wait_range(mapping, iocb->ki_pos, end);
if (ret)
Expand All @@ -299,7 +265,7 @@ xfs_file_dio_aio_read(
}

out_unlock:
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
return ret;
}

Expand All @@ -317,9 +283,9 @@ xfs_file_dax_read(
if (!count)
return 0; /* skip atime */

xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
xfs_ilock(ip, XFS_IOLOCK_SHARED);
ret = dax_iomap_rw(iocb, to, &xfs_iomap_ops);
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
xfs_iunlock(ip, XFS_IOLOCK_SHARED);

file_accessed(iocb->ki_filp);
return ret;
Expand All @@ -335,9 +301,9 @@ xfs_file_buffered_aio_read(

trace_xfs_file_buffered_read(ip, iov_iter_count(to), iocb->ki_pos);

xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
xfs_ilock(ip, XFS_IOLOCK_SHARED);
ret = generic_file_read_iter(iocb, to);
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
xfs_iunlock(ip, XFS_IOLOCK_SHARED);

return ret;
}
Expand Down Expand Up @@ -418,15 +384,18 @@ xfs_file_aio_write_checks(
if (error <= 0)
return error;

error = xfs_break_layouts(inode, iolock, true);
error = xfs_break_layouts(inode, iolock);
if (error)
return error;

/* For changing security info in file_remove_privs() we need i_mutex */
/*
* For changing security info in file_remove_privs() we need i_rwsem
* exclusively.
*/
if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
xfs_rw_iunlock(ip, *iolock);
xfs_iunlock(ip, *iolock);
*iolock = XFS_IOLOCK_EXCL;
xfs_rw_ilock(ip, *iolock);
xfs_ilock(ip, *iolock);
goto restart;
}
/*
Expand All @@ -451,9 +420,9 @@ xfs_file_aio_write_checks(
spin_unlock(&ip->i_flags_lock);
if (!drained_dio) {
if (*iolock == XFS_IOLOCK_SHARED) {
xfs_rw_iunlock(ip, *iolock);
xfs_iunlock(ip, *iolock);
*iolock = XFS_IOLOCK_EXCL;
xfs_rw_ilock(ip, *iolock);
xfs_ilock(ip, *iolock);
iov_iter_reexpand(from, count);
}
/*
Expand Down Expand Up @@ -559,7 +528,7 @@ xfs_file_dio_aio_write(
iolock = XFS_IOLOCK_SHARED;
}

xfs_rw_ilock(ip, iolock);
xfs_ilock(ip, iolock);

ret = xfs_file_aio_write_checks(iocb, from, &iolock);
if (ret)
Expand Down Expand Up @@ -591,7 +560,7 @@ xfs_file_dio_aio_write(
if (unaligned_io)
inode_dio_wait(inode);
else if (iolock == XFS_IOLOCK_EXCL) {
xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
iolock = XFS_IOLOCK_SHARED;
}

Expand Down Expand Up @@ -621,7 +590,7 @@ xfs_file_dio_aio_write(
iov_iter_advance(from, ret);
}
out:
xfs_rw_iunlock(ip, iolock);
xfs_iunlock(ip, iolock);

/*
* No fallback to buffered IO on errors for XFS, direct IO will either
Expand All @@ -643,7 +612,7 @@ xfs_file_dax_write(
size_t count;
loff_t pos;

xfs_rw_ilock(ip, iolock);
xfs_ilock(ip, iolock);
ret = xfs_file_aio_write_checks(iocb, from, &iolock);
if (ret)
goto out;
Expand All @@ -652,15 +621,13 @@ xfs_file_dax_write(
count = iov_iter_count(from);

trace_xfs_file_dax_write(ip, count, pos);

ret = dax_iomap_rw(iocb, from, &xfs_iomap_ops);
if (ret > 0 && iocb->ki_pos > i_size_read(inode)) {
i_size_write(inode, iocb->ki_pos);
error = xfs_setfilesize(ip, pos, ret);
}

out:
xfs_rw_iunlock(ip, iolock);
xfs_iunlock(ip, iolock);
return error ? error : ret;
}

Expand All @@ -677,7 +644,7 @@ xfs_file_buffered_aio_write(
int enospc = 0;
int iolock = XFS_IOLOCK_EXCL;

xfs_rw_ilock(ip, iolock);
xfs_ilock(ip, iolock);

ret = xfs_file_aio_write_checks(iocb, from, &iolock);
if (ret)
Expand Down Expand Up @@ -721,7 +688,7 @@ xfs_file_buffered_aio_write(

current->backing_dev_info = NULL;
out:
xfs_rw_iunlock(ip, iolock);
xfs_iunlock(ip, iolock);
return ret;
}

Expand Down Expand Up @@ -797,7 +764,7 @@ xfs_file_fallocate(
return -EOPNOTSUPP;

xfs_ilock(ip, iolock);
error = xfs_break_layouts(inode, &iolock, false);
error = xfs_break_layouts(inode, &iolock);
if (error)
goto out_unlock;

Expand Down
6 changes: 2 additions & 4 deletions fs/xfs/xfs_icache.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,6 @@ xfs_inode_alloc(
ASSERT(!xfs_isiflocked(ip));
ASSERT(ip->i_ino == 0);

mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);

/* initialise the xfs inode */
ip->i_ino = ino;
ip->i_mount = mp;
Expand Down Expand Up @@ -394,8 +392,8 @@ xfs_iget_cache_hit(
xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
inode->i_state = I_NEW;

ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));
mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);
ASSERT(!rwsem_is_locked(&inode->i_rwsem));
init_rwsem(&inode->i_rwsem);

spin_unlock(&ip->i_flags_lock);
spin_unlock(&pag->pag_ici_lock);
Expand Down
Loading

0 comments on commit 6552321

Please sign in to comment.