| Message ID | 20170224162043.988779074@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Feb 24, 2017 at 08:56:06AM -0800, Linus Torvalds wrote: > This one looks very questionable. > > Taking the lock for every single inode, even when we can tell that it's > pointless, is horrid. > > Even if you really think you found a race, i think it would be better to > leave the unlocked read around as an optimistic check, and then do a safe > double check read inside the lock (together with the i_state checks) > > Hmm? Yeah, pretty dumb that.
--- a/fs/inode.c +++ b/fs/inode.c @@ -604,10 +604,12 @@ void evict_inodes(struct super_block *sb again: spin_lock(&sb->s_inode_list_lock); list_for_each_entry_safe(inode, next, &sb->s_inodes, i_sb_list) { - if (atomic_read(&inode->i_count)) + spin_lock(&inode->i_lock); + if (atomic_read(&inode->i_count)) { + spin_unlock(&inode->i_lock); continue; + } - spin_lock(&inode->i_lock); if (inode->i_state & (I_NEW | I_FREEING | I_WILL_FREE)) { spin_unlock(&inode->i_lock); continue;
In general i_count will only stay 0 if we have i_lock held. I realize this is evict, so having MS_ACTIVE cleared might avoid the race against find_inode_fast() in other ways. Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> --- fs/inode.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)