[PATCH v2] vfs: don't softlockup when evicting inodes

[PATCH v2] vfs: don't softlockup when evicting inodes

[Josef Bacik]

If I run an fs_mark job that creates millions of empty files and then
immediately unmount the file system I will get a softlockup during unmount.
This box has ~140gb of RAM so we never hit sufficient memory pressure to evict
enough inodes during the runtime of the benchmark, which means I see around 80
million inodes being evicted at unmount time.  With this patch my box no longer
softlocks up.  Thanks,

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
V1->V2:
-Still occasionally saw a softlockup in evict_inodes so add a cond_resched_lock
 to that case as well.

 fs/inode.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/inode.c b/fs/inode.c
index ad60555..f266765 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -581,6 +581,7 @@ static void dispose_list(struct list_head *head)
 		list_del_init(&inode->i_lru);
 
 		evict(inode);
+		cond_resched();
 	}
 }
 
@@ -613,6 +614,7 @@ void evict_inodes(struct super_block *sb)
 		inode_lru_list_del(inode);
 		spin_unlock(&inode->i_lock);
 		list_add(&inode->i_lru, &dispose);
+		cond_resched_lock(&inode_sb_list_lock);
 	}
 	spin_unlock(&inode_sb_list_lock);
 
-- 
1.9.3

Re: [PATCH v2] vfs: don't softlockup when evicting inodes

[Jeff Layton]

On Fri, 19 Dec 2014 10:34:40 -0500
Josef Bacik <jbacik@fb.com> wrote:

> If I run an fs_mark job that creates millions of empty files and then
> immediately unmount the file system I will get a softlockup during unmount.
> This box has ~140gb of RAM so we never hit sufficient memory pressure to evict
> enough inodes during the runtime of the benchmark, which means I see around 80
> million inodes being evicted at unmount time.  With this patch my box no longer
> softlocks up.  Thanks,
> 
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> V1->V2:
> -Still occasionally saw a softlockup in evict_inodes so add a cond_resched_lock
>  to that case as well.
> 
>  fs/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index ad60555..f266765 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -581,6 +581,7 @@ static void dispose_list(struct list_head *head)
>  		list_del_init(&inode->i_lru);
>  
>  		evict(inode);
> +		cond_resched();
>  	}
>  }
>  
> @@ -613,6 +614,7 @@ void evict_inodes(struct super_block *sb)
>  		inode_lru_list_del(inode);
>  		spin_unlock(&inode->i_lock);
>  		list_add(&inode->i_lru, &dispose);
> +		cond_resched_lock(&inode_sb_list_lock);
>  	}
>  	spin_unlock(&inode_sb_list_lock);
>  

Is that safe? What guarantees that the next entry in the list (that is,
the one already prefetched by list_for_each_entry_safe) will still be
there once you drop and reacquire the lock?

-- 
Jeff Layton <jlayton@primarydata.com>

Re: [PATCH v2] vfs: don't softlockup when evicting inodes

[Josef Bacik]

I had convinced myself it was fine but if we have a final iput of a deleted inode it could screw us, I'll fix this to restart the loop if we resched.  Thanks,

Josef

Jeff Layton <jeff.layton@primarydata.com> wrote:


On Fri, 19 Dec 2014 10:34:40 -0500
Josef Bacik <jbacik@fb.com> wrote:

> If I run an fs_mark job that creates millions of empty files and then
> immediately unmount the file system I will get a softlockup during unmount.
> This box has ~140gb of RAM so we never hit sufficient memory pressure to evict
> enough inodes during the runtime of the benchmark, which means I see around 80
> million inodes being evicted at unmount time.  With this patch my box no longer
> softlocks up.  Thanks,
>
> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
> V1->V2:
> -Still occasionally saw a softlockup in evict_inodes so add a cond_resched_lock
>  to that case as well.
>
>  fs/inode.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/fs/inode.c b/fs/inode.c
> index ad60555..f266765 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -581,6 +581,7 @@ static void dispose_list(struct list_head *head)
>               list_del_init(&inode->i_lru);
>
>               evict(inode);
> +             cond_resched();
>       }
>  }
>
> @@ -613,6 +614,7 @@ void evict_inodes(struct super_block *sb)
>               inode_lru_list_del(inode);
>               spin_unlock(&inode->i_lock);
>               list_add(&inode->i_lru, &dispose);
> +             cond_resched_lock(&inode_sb_list_lock);
>       }
>       spin_unlock(&inode_sb_list_lock);
>

Is that safe? What guarantees that the next entry in the list (that is,
the one already prefetched by list_for_each_entry_safe) will still be
there once you drop and reacquire the lock?

--
Jeff Layton <jlayton@primarydata.com>