Skip to content

Commit 6552321

Browse files
Christoph Hellwigdchinner
Christoph Hellwig
authored and
dchinner
committed
xfs: remove i_iolock and use i_rwsem in the VFS inode instead
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>
1 parent f831948 commit 6552321

14 files changed

+86
-159
lines changed

‎fs/xfs/xfs_aops.c

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,20 +1585,17 @@ xfs_vm_bmap(
15851585
struct xfs_inode *ip = XFS_I(inode);
15861586

15871587
trace_xfs_vm_bmap(XFS_I(inode));
1588-
xfs_ilock(ip, XFS_IOLOCK_SHARED);
15891588

15901589
/*
15911590
* The swap code (ab-)uses ->bmap to get a block mapping and then
15921591
* bypasseѕ the file system for actual I/O. We really can't allow
15931592
* that on reflinks inodes, so we have to skip out here. And yes,
15941593
* 0 is the magic code for a bmap error..
15951594
*/
1596-
if (xfs_is_reflink_inode(ip)) {
1597-
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
1595+
if (xfs_is_reflink_inode(ip))
15981596
return 0;
1599-
}
1597+
16001598
filemap_write_and_wait(mapping);
1601-
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
16021599
return generic_block_bmap(mapping, block, xfs_get_blocks);
16031600
}
16041601

‎fs/xfs/xfs_bmap_util.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1935,8 +1935,8 @@ xfs_swap_extents(
19351935
* page cache safely. Once we have done this we can take the ilocks and
19361936
* do the rest of the checks.
19371937
*/
1938-
lock_flags = XFS_IOLOCK_EXCL | XFS_MMAPLOCK_EXCL;
1939-
xfs_lock_two_inodes(ip, tip, XFS_IOLOCK_EXCL);
1938+
lock_two_nondirectories(VFS_I(ip), VFS_I(tip));
1939+
lock_flags = XFS_MMAPLOCK_EXCL;
19401940
xfs_lock_two_inodes(ip, tip, XFS_MMAPLOCK_EXCL);
19411941

19421942
/* Verify that both files have the same format */
@@ -2076,15 +2076,13 @@ xfs_swap_extents(
20762076
trace_xfs_swap_extent_after(ip, 0);
20772077
trace_xfs_swap_extent_after(tip, 1);
20782078

2079+
out_unlock:
20792080
xfs_iunlock(ip, lock_flags);
20802081
xfs_iunlock(tip, lock_flags);
2082+
unlock_two_nondirectories(VFS_I(ip), VFS_I(tip));
20812083
return error;
20822084

20832085
out_trans_cancel:
20842086
xfs_trans_cancel(tp);
2085-
2086-
out_unlock:
2087-
xfs_iunlock(ip, lock_flags);
2088-
xfs_iunlock(tip, lock_flags);
2089-
return error;
2087+
goto out_unlock;
20902088
}

‎fs/xfs/xfs_dir2_readdir.c

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -677,7 +677,6 @@ xfs_readdir(
677677
args.dp = dp;
678678
args.geo = dp->i_mount->m_dir_geo;
679679

680-
xfs_ilock(dp, XFS_IOLOCK_SHARED);
681680
if (dp->i_d.di_format == XFS_DINODE_FMT_LOCAL)
682681
rval = xfs_dir2_sf_getdents(&args, ctx);
683682
else if ((rval = xfs_dir2_isblock(&args, &v)))
@@ -686,7 +685,6 @@ xfs_readdir(
686685
rval = xfs_dir2_block_getdents(&args, ctx);
687686
else
688687
rval = xfs_dir2_leaf_getdents(&args, ctx, bufsize);
689-
xfs_iunlock(dp, XFS_IOLOCK_SHARED);
690688

691689
return rval;
692690
}

‎fs/xfs/xfs_file.c

Lines changed: 23 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -47,40 +47,6 @@
4747

4848
static const struct vm_operations_struct xfs_file_vm_ops;
4949

50-
/*
51-
* Locking primitives for read and write IO paths to ensure we consistently use
52-
* and order the inode->i_mutex, ip->i_lock and ip->i_iolock.
53-
*/
54-
static inline void
55-
xfs_rw_ilock(
56-
struct xfs_inode *ip,
57-
int type)
58-
{
59-
if (type & XFS_IOLOCK_EXCL)
60-
inode_lock(VFS_I(ip));
61-
xfs_ilock(ip, type);
62-
}
63-
64-
static inline void
65-
xfs_rw_iunlock(
66-
struct xfs_inode *ip,
67-
int type)
68-
{
69-
xfs_iunlock(ip, type);
70-
if (type & XFS_IOLOCK_EXCL)
71-
inode_unlock(VFS_I(ip));
72-
}
73-
74-
static inline void
75-
xfs_rw_ilock_demote(
76-
struct xfs_inode *ip,
77-
int type)
78-
{
79-
xfs_ilock_demote(ip, type);
80-
if (type & XFS_IOLOCK_EXCL)
81-
inode_unlock(VFS_I(ip));
82-
}
83-
8450
/*
8551
* Clear the specified ranges to zero through either the pagecache or DAX.
8652
* Holes and unwritten extents will be left as-is as they already are zeroed.
@@ -273,7 +239,7 @@ xfs_file_dio_aio_read(
273239

274240
file_accessed(iocb->ki_filp);
275241

276-
xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
242+
xfs_ilock(ip, XFS_IOLOCK_SHARED);
277243
if (mapping->nrpages) {
278244
ret = filemap_write_and_wait_range(mapping, iocb->ki_pos, end);
279245
if (ret)
@@ -299,7 +265,7 @@ xfs_file_dio_aio_read(
299265
}
300266

301267
out_unlock:
302-
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
268+
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
303269
return ret;
304270
}
305271

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

320-
xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
286+
xfs_ilock(ip, XFS_IOLOCK_SHARED);
321287
ret = dax_iomap_rw(iocb, to, &xfs_iomap_ops);
322-
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
288+
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
323289

324290
file_accessed(iocb->ki_filp);
325291
return ret;
@@ -335,9 +301,9 @@ xfs_file_buffered_aio_read(
335301

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

338-
xfs_rw_ilock(ip, XFS_IOLOCK_SHARED);
304+
xfs_ilock(ip, XFS_IOLOCK_SHARED);
339305
ret = generic_file_read_iter(iocb, to);
340-
xfs_rw_iunlock(ip, XFS_IOLOCK_SHARED);
306+
xfs_iunlock(ip, XFS_IOLOCK_SHARED);
341307

342308
return ret;
343309
}
@@ -418,15 +384,18 @@ xfs_file_aio_write_checks(
418384
if (error <= 0)
419385
return error;
420386

421-
error = xfs_break_layouts(inode, iolock, true);
387+
error = xfs_break_layouts(inode, iolock);
422388
if (error)
423389
return error;
424390

425-
/* For changing security info in file_remove_privs() we need i_mutex */
391+
/*
392+
* For changing security info in file_remove_privs() we need i_rwsem
393+
* exclusively.
394+
*/
426395
if (*iolock == XFS_IOLOCK_SHARED && !IS_NOSEC(inode)) {
427-
xfs_rw_iunlock(ip, *iolock);
396+
xfs_iunlock(ip, *iolock);
428397
*iolock = XFS_IOLOCK_EXCL;
429-
xfs_rw_ilock(ip, *iolock);
398+
xfs_ilock(ip, *iolock);
430399
goto restart;
431400
}
432401
/*
@@ -451,9 +420,9 @@ xfs_file_aio_write_checks(
451420
spin_unlock(&ip->i_flags_lock);
452421
if (!drained_dio) {
453422
if (*iolock == XFS_IOLOCK_SHARED) {
454-
xfs_rw_iunlock(ip, *iolock);
423+
xfs_iunlock(ip, *iolock);
455424
*iolock = XFS_IOLOCK_EXCL;
456-
xfs_rw_ilock(ip, *iolock);
425+
xfs_ilock(ip, *iolock);
457426
iov_iter_reexpand(from, count);
458427
}
459428
/*
@@ -559,7 +528,7 @@ xfs_file_dio_aio_write(
559528
iolock = XFS_IOLOCK_SHARED;
560529
}
561530

562-
xfs_rw_ilock(ip, iolock);
531+
xfs_ilock(ip, iolock);
563532

564533
ret = xfs_file_aio_write_checks(iocb, from, &iolock);
565534
if (ret)
@@ -591,7 +560,7 @@ xfs_file_dio_aio_write(
591560
if (unaligned_io)
592561
inode_dio_wait(inode);
593562
else if (iolock == XFS_IOLOCK_EXCL) {
594-
xfs_rw_ilock_demote(ip, XFS_IOLOCK_EXCL);
563+
xfs_ilock_demote(ip, XFS_IOLOCK_EXCL);
595564
iolock = XFS_IOLOCK_SHARED;
596565
}
597566

@@ -621,7 +590,7 @@ xfs_file_dio_aio_write(
621590
iov_iter_advance(from, ret);
622591
}
623592
out:
624-
xfs_rw_iunlock(ip, iolock);
593+
xfs_iunlock(ip, iolock);
625594

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

646-
xfs_rw_ilock(ip, iolock);
615+
xfs_ilock(ip, iolock);
647616
ret = xfs_file_aio_write_checks(iocb, from, &iolock);
648617
if (ret)
649618
goto out;
@@ -652,15 +621,13 @@ xfs_file_dax_write(
652621
count = iov_iter_count(from);
653622

654623
trace_xfs_file_dax_write(ip, count, pos);
655-
656624
ret = dax_iomap_rw(iocb, from, &xfs_iomap_ops);
657625
if (ret > 0 && iocb->ki_pos > i_size_read(inode)) {
658626
i_size_write(inode, iocb->ki_pos);
659627
error = xfs_setfilesize(ip, pos, ret);
660628
}
661-
662629
out:
663-
xfs_rw_iunlock(ip, iolock);
630+
xfs_iunlock(ip, iolock);
664631
return error ? error : ret;
665632
}
666633

@@ -677,7 +644,7 @@ xfs_file_buffered_aio_write(
677644
int enospc = 0;
678645
int iolock = XFS_IOLOCK_EXCL;
679646

680-
xfs_rw_ilock(ip, iolock);
647+
xfs_ilock(ip, iolock);
681648

682649
ret = xfs_file_aio_write_checks(iocb, from, &iolock);
683650
if (ret)
@@ -721,7 +688,7 @@ xfs_file_buffered_aio_write(
721688

722689
current->backing_dev_info = NULL;
723690
out:
724-
xfs_rw_iunlock(ip, iolock);
691+
xfs_iunlock(ip, iolock);
725692
return ret;
726693
}
727694

@@ -797,7 +764,7 @@ xfs_file_fallocate(
797764
return -EOPNOTSUPP;
798765

799766
xfs_ilock(ip, iolock);
800-
error = xfs_break_layouts(inode, &iolock, false);
767+
error = xfs_break_layouts(inode, &iolock);
801768
if (error)
802769
goto out_unlock;
803770

‎fs/xfs/xfs_icache.c

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,8 +70,6 @@ xfs_inode_alloc(
7070
ASSERT(!xfs_isiflocked(ip));
7171
ASSERT(ip->i_ino == 0);
7272

73-
mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);
74-
7573
/* initialise the xfs inode */
7674
ip->i_ino = ino;
7775
ip->i_mount = mp;
@@ -394,8 +392,8 @@ xfs_iget_cache_hit(
394392
xfs_inode_clear_reclaim_tag(pag, ip->i_ino);
395393
inode->i_state = I_NEW;
396394

397-
ASSERT(!rwsem_is_locked(&ip->i_iolock.mr_lock));
398-
mrlock_init(&ip->i_iolock, MRLOCK_BARRIER, "xfsio", ip->i_ino);
395+
ASSERT(!rwsem_is_locked(&inode->i_rwsem));
396+
init_rwsem(&inode->i_rwsem);
399397

400398
spin_unlock(&ip->i_flags_lock);
401399
spin_unlock(&pag->pag_ici_lock);

0 commit comments

Comments
 (0)