* [PATCH 0/2] avoid softlockups in various s_inodes iterators
@ 2019-11-07 19:42 Eric Sandeen
2019-11-07 19:47 ` [PATCH 1/2] fs: avoid softlockups in " Eric Sandeen
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Eric Sandeen @ 2019-11-07 19:42 UTC (permalink / raw)
To: fsdevel; +Cc: Al Viro
2 patches to make sure we either schedule in an s_inodes walking
loop, or do our best to limit the size of the walk, to avoid soft
lockups.
-Eric
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] fs: avoid softlockups in s_inodes iterators
2019-11-07 19:42 [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
@ 2019-11-07 19:47 ` Eric Sandeen
2019-11-07 19:48 ` [PATCH 2/2] fs: call fsnotify_sb_delete after evict_inodes Eric Sandeen
2019-11-07 19:58 ` [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
2 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2019-11-07 19:47 UTC (permalink / raw)
To: fsdevel; +Cc: Al Viro
Anything that walks all inodes on sb->s_inodes list without rescheduling
risks softlockups.
Previous efforts were made in 2 functions, see:
c27d82f fs/drop_caches.c: avoid softlockups in drop_pagecache_sb()
ac05fbb inode: don't softlockup when evicting inodes
but there hasn't been an audit of all walkers, so do that now. This
also consistently moves the cond_resched() calls to the bottom of each
loop in cases where it already exists.
One loop remains: remove_dquot_ref(), because I'm not quite sure how
to deal with that one w/o taking the i_lock.
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
V2: Drop unrelated iput cleanups in fsnotify
diff --git a/fs/drop_caches.c b/fs/drop_caches.c
index d31b6c72b476..dc1a1d5d825b 100644
--- a/fs/drop_caches.c
+++ b/fs/drop_caches.c
@@ -35,11 +35,11 @@ static void drop_pagecache_sb(struct super_block *sb, void *unused)
spin_unlock(&inode->i_lock);
spin_unlock(&sb->s_inode_list_lock);
- cond_resched();
invalidate_mapping_pages(inode->i_mapping, 0, -1);
iput(toput_inode);
toput_inode = inode;
+ cond_resched();
spin_lock(&sb->s_inode_list_lock);
}
spin_unlock(&sb->s_inode_list_lock);
diff --git a/fs/inode.c b/fs/inode.c
index fef457a42882..b0c789bb3dba 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -676,6 +676,7 @@ int invalidate_inodes(struct super_block *sb, bool kill_dirty)
struct inode *inode, *next;
LIST_HEAD(dispose);
+again:
spin_lock(&sb->s_inode_list_lock);
list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) {
spin_lock(&inode->i_lock);
@@ -698,6 +699,13 @@ int invalidate_inodes(struct super_block *sb, bool kill_dirty)
inode_lru_list_del(inode);
spin_unlock(&inode->i_lock);
list_add(&inode->i_lru, &dispose);
+
+ if (need_resched()) {
+ spin_unlock(&sb->s_inode_list_lock);
+ cond_resched();
+ dispose_list(&dispose);
+ goto again;
+ }
}
spin_unlock(&sb->s_inode_list_lock);
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 2ecef6155fc0..ac9eb273e28c 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -77,6 +77,7 @@ static void fsnotify_unmount_inodes(struct super_block *sb)
iput_inode = inode;
+ cond_resched();
spin_lock(&sb->s_inode_list_lock);
}
spin_unlock(&sb->s_inode_list_lock);
diff --git a/fs/quota/dquot.c b/fs/quota/dquot.c
index 6e826b454082..4a085b3c7cac 100644
--- a/fs/quota/dquot.c
+++ b/fs/quota/dquot.c
@@ -985,6 +985,7 @@ static int add_dquot_ref(struct super_block *sb, int type)
* later.
*/
old_inode = inode;
+ cond_resched();
spin_lock(&sb->s_inode_list_lock);
}
spin_unlock(&sb->s_inode_list_lock);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH 2/2] fs: call fsnotify_sb_delete after evict_inodes
2019-11-07 19:42 [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
2019-11-07 19:47 ` [PATCH 1/2] fs: avoid softlockups in " Eric Sandeen
@ 2019-11-07 19:48 ` Eric Sandeen
2019-11-07 19:58 ` [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
2 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2019-11-07 19:48 UTC (permalink / raw)
To: fsdevel; +Cc: Al Viro
When a filesystem is unmounted, we currently call fsnotify_sb_delete()
before evict_inodes(), which means that fsnotify_unmount_inodes()
must iterate over all inodes on the superblock, even though it will
only act on inodes with a refcount. This is inefficient and can lead
to livelocks as it iterates over many unrefcounted inodes.
However, since fsnotify_sb_delete() and evict_inodes() are working
on orthogonal sets of inodes (fsnotify_sb_delete() only cares about
nonzero refcount, and evict_inodes() only cares about zero refcount),
we can swap the order of the calls. The fsnotify call will then have
a much smaller list to walk (any refcounted inodes).
This should speed things up overall, and avoid livelocks in
fsnotify_unmount_inodes().
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Jan Kara <jack@suse.cz>
---
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index ac9eb273e28c..426f03b6e660 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -57,6 +57,10 @@ static void fsnotify_unmount_inodes(struct super_block *sb)
* doing an __iget/iput with SB_ACTIVE clear would actually
* evict all inodes with zero i_count from icache which is
* unnecessarily violent and may in fact be illegal to do.
+ *
+ * However, we should have been called /after/ evict_inodes
+ * removed all zero refcount inodes, in any case. Test to
+ * be sure.
*/
if (!atomic_read(&inode->i_count)) {
spin_unlock(&inode->i_lock);
diff --git a/fs/super.c b/fs/super.c
index cfadab2cbf35..cd352530eca9 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -448,10 +448,12 @@ void generic_shutdown_super(struct super_block *sb)
sync_filesystem(sb);
sb->s_flags &= ~SB_ACTIVE;
- fsnotify_sb_delete(sb);
cgroup_writeback_umount();
+ /* evict all inodes with zero refcount */
evict_inodes(sb);
+ /* only nonzero refcount inodes can have marks */
+ fsnotify_sb_delete(sb);
if (sb->s_dio_done_wq) {
destroy_workqueue(sb->s_dio_done_wq);
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] avoid softlockups in various s_inodes iterators
2019-11-07 19:42 [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
2019-11-07 19:47 ` [PATCH 1/2] fs: avoid softlockups in " Eric Sandeen
2019-11-07 19:48 ` [PATCH 2/2] fs: call fsnotify_sb_delete after evict_inodes Eric Sandeen
@ 2019-11-07 19:58 ` Eric Sandeen
2 siblings, 0 replies; 4+ messages in thread
From: Eric Sandeen @ 2019-11-07 19:58 UTC (permalink / raw)
To: fsdevel; +Cc: Al Viro
On 11/7/19 1:42 PM, Eric Sandeen wrote:
> 2 patches to make sure we either schedule in an s_inodes walking
> loop, or do our best to limit the size of the walk, to avoid soft
> lockups.
>
> -Eric
>
Something mangled that sorry, let me resend it sanely...
-Eric
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-11-07 19:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-07 19:42 [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
2019-11-07 19:47 ` [PATCH 1/2] fs: avoid softlockups in " Eric Sandeen
2019-11-07 19:48 ` [PATCH 2/2] fs: call fsnotify_sb_delete after evict_inodes Eric Sandeen
2019-11-07 19:58 ` [PATCH 0/2] avoid softlockups in various s_inodes iterators Eric Sandeen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).