Skip to content

Commit f01c30d

Browse files
committed
Merge tag 'vfs-5.10-fixes-2' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux
Pull fs freeze fix and cleanups from Darrick Wong: "A single vfs fix for 5.10, along with two subsequent cleanups. A very long time ago, a hack was added to the vfs fs freeze protection code to work around lockdep complaints about XFS, which would try to run a transaction (which requires intwrite protection) to finalize an xfs freeze (by which time the vfs had already taken intwrite). Fast forward a few years, and XFS fixed the recursive intwrite problem on its own, and the hack became unnecessary. Fast forward almost a decade, and latent bugs in the code converting this hack from freeze flags to freeze locks combine with lockdep bugs to make this reproduce frequently enough to notice page faults racing with freeze. Since the hack is unnecessary and causes thread race errors, just get rid of it completely. Making this kind of vfs change midway through a cycle makes me nervous, but a large enough number of the usual VFS/ext4/XFS/btrfs suspects have said this looks good and solves a real problem vector. And once that removal is done, __sb_start_write is now simple enough that it becomes possible to refactor the function into smaller, simpler static inline helpers in linux/fs.h. The cleanup is straightforward. Summary: - Finally remove the "convert to trylock" weirdness in the fs freezer code. It was necessary 10 years ago to deal with nested transactions in XFS, but we've long since removed that; and now this is causing subtle race conditions when lockdep goes offline and sb_start_* aren't prepared to retry a trylock failure. - Minor cleanups of the sb_start_* fs freeze helpers" * tag 'vfs-5.10-fixes-2' of git://git.kernel.org/pub/scm/fs/xfs/xfs-linux: vfs: move __sb_{start,end}_write* to fs.h vfs: separate __sb_start_write into blocking and non-blocking helpers vfs: remove lockdep bogosity in __sb_start_write
2 parents d9315f5 + 9b85234 commit f01c30d

File tree

4 files changed

+29
-63
lines changed

4 files changed

+29
-63
lines changed

fs/aio.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1572,7 +1572,7 @@ static int aio_write(struct kiocb *req, const struct iocb *iocb,
15721572
* we return to userspace.
15731573
*/
15741574
if (S_ISREG(file_inode(file)->i_mode)) {
1575-
__sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, true);
1575+
sb_start_write(file_inode(file)->i_sb);
15761576
__sb_writers_release(file_inode(file)->i_sb, SB_FREEZE_WRITE);
15771577
}
15781578
req->ki_flags |= IOCB_WRITE;

fs/io_uring.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3547,8 +3547,7 @@ static int io_write(struct io_kiocb *req, bool force_nonblock,
35473547
* we return to userspace.
35483548
*/
35493549
if (req->flags & REQ_F_ISREG) {
3550-
__sb_start_write(file_inode(req->file)->i_sb,
3551-
SB_FREEZE_WRITE, true);
3550+
sb_start_write(file_inode(req->file)->i_sb);
35523551
__sb_writers_release(file_inode(req->file)->i_sb,
35533552
SB_FREEZE_WRITE);
35543553
}

fs/super.c

Lines changed: 0 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1631,55 +1631,6 @@ int super_setup_bdi(struct super_block *sb)
16311631
}
16321632
EXPORT_SYMBOL(super_setup_bdi);
16331633

1634-
/*
1635-
* This is an internal function, please use sb_end_{write,pagefault,intwrite}
1636-
* instead.
1637-
*/
1638-
void __sb_end_write(struct super_block *sb, int level)
1639-
{
1640-
percpu_up_read(sb->s_writers.rw_sem + level-1);
1641-
}
1642-
EXPORT_SYMBOL(__sb_end_write);
1643-
1644-
/*
1645-
* This is an internal function, please use sb_start_{write,pagefault,intwrite}
1646-
* instead.
1647-
*/
1648-
int __sb_start_write(struct super_block *sb, int level, bool wait)
1649-
{
1650-
bool force_trylock = false;
1651-
int ret = 1;
1652-
1653-
#ifdef CONFIG_LOCKDEP
1654-
/*
1655-
* We want lockdep to tell us about possible deadlocks with freezing
1656-
* but it's it bit tricky to properly instrument it. Getting a freeze
1657-
* protection works as getting a read lock but there are subtle
1658-
* problems. XFS for example gets freeze protection on internal level
1659-
* twice in some cases, which is OK only because we already hold a
1660-
* freeze protection also on higher level. Due to these cases we have
1661-
* to use wait == F (trylock mode) which must not fail.
1662-
*/
1663-
if (wait) {
1664-
int i;
1665-
1666-
for (i = 0; i < level - 1; i++)
1667-
if (percpu_rwsem_is_held(sb->s_writers.rw_sem + i)) {
1668-
force_trylock = true;
1669-
break;
1670-
}
1671-
}
1672-
#endif
1673-
if (wait && !force_trylock)
1674-
percpu_down_read(sb->s_writers.rw_sem + level-1);
1675-
else
1676-
ret = percpu_down_read_trylock(sb->s_writers.rw_sem + level-1);
1677-
1678-
WARN_ON(force_trylock && !ret);
1679-
return ret;
1680-
}
1681-
EXPORT_SYMBOL(__sb_start_write);
1682-
16831634
/**
16841635
* sb_wait_write - wait until all writers to given file system finish
16851636
* @sb: the super for which we wait

include/linux/fs.h

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1580,8 +1580,24 @@ extern struct timespec64 current_time(struct inode *inode);
15801580
* Snapshotting support.
15811581
*/
15821582

1583-
void __sb_end_write(struct super_block *sb, int level);
1584-
int __sb_start_write(struct super_block *sb, int level, bool wait);
1583+
/*
1584+
* These are internal functions, please use sb_start_{write,pagefault,intwrite}
1585+
* instead.
1586+
*/
1587+
static inline void __sb_end_write(struct super_block *sb, int level)
1588+
{
1589+
percpu_up_read(sb->s_writers.rw_sem + level-1);
1590+
}
1591+
1592+
static inline void __sb_start_write(struct super_block *sb, int level)
1593+
{
1594+
percpu_down_read(sb->s_writers.rw_sem + level - 1);
1595+
}
1596+
1597+
static inline bool __sb_start_write_trylock(struct super_block *sb, int level)
1598+
{
1599+
return percpu_down_read_trylock(sb->s_writers.rw_sem + level - 1);
1600+
}
15851601

15861602
#define __sb_writers_acquired(sb, lev) \
15871603
percpu_rwsem_acquire(&(sb)->s_writers.rw_sem[(lev)-1], 1, _THIS_IP_)
@@ -1645,12 +1661,12 @@ static inline void sb_end_intwrite(struct super_block *sb)
16451661
*/
16461662
static inline void sb_start_write(struct super_block *sb)
16471663
{
1648-
__sb_start_write(sb, SB_FREEZE_WRITE, true);
1664+
__sb_start_write(sb, SB_FREEZE_WRITE);
16491665
}
16501666

1651-
static inline int sb_start_write_trylock(struct super_block *sb)
1667+
static inline bool sb_start_write_trylock(struct super_block *sb)
16521668
{
1653-
return __sb_start_write(sb, SB_FREEZE_WRITE, false);
1669+
return __sb_start_write_trylock(sb, SB_FREEZE_WRITE);
16541670
}
16551671

16561672
/**
@@ -1674,7 +1690,7 @@ static inline int sb_start_write_trylock(struct super_block *sb)
16741690
*/
16751691
static inline void sb_start_pagefault(struct super_block *sb)
16761692
{
1677-
__sb_start_write(sb, SB_FREEZE_PAGEFAULT, true);
1693+
__sb_start_write(sb, SB_FREEZE_PAGEFAULT);
16781694
}
16791695

16801696
/*
@@ -1692,12 +1708,12 @@ static inline void sb_start_pagefault(struct super_block *sb)
16921708
*/
16931709
static inline void sb_start_intwrite(struct super_block *sb)
16941710
{
1695-
__sb_start_write(sb, SB_FREEZE_FS, true);
1711+
__sb_start_write(sb, SB_FREEZE_FS);
16961712
}
16971713

1698-
static inline int sb_start_intwrite_trylock(struct super_block *sb)
1714+
static inline bool sb_start_intwrite_trylock(struct super_block *sb)
16991715
{
1700-
return __sb_start_write(sb, SB_FREEZE_FS, false);
1716+
return __sb_start_write_trylock(sb, SB_FREEZE_FS);
17011717
}
17021718

17031719

@@ -2756,14 +2772,14 @@ static inline void file_start_write(struct file *file)
27562772
{
27572773
if (!S_ISREG(file_inode(file)->i_mode))
27582774
return;
2759-
__sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, true);
2775+
sb_start_write(file_inode(file)->i_sb);
27602776
}
27612777

27622778
static inline bool file_start_write_trylock(struct file *file)
27632779
{
27642780
if (!S_ISREG(file_inode(file)->i_mode))
27652781
return true;
2766-
return __sb_start_write(file_inode(file)->i_sb, SB_FREEZE_WRITE, false);
2782+
return sb_start_write_trylock(file_inode(file)->i_sb);
27672783
}
27682784

27692785
static inline void file_end_write(struct file *file)

0 commit comments

Comments
 (0)