On 2020/9/19 22:56, Matthew Wilcox wrote:
This part is unnecessary. We just allocated 'new' two lines above;
nobody else can see 'new' yet. We make it visible with hlist_add_head_rcu()
which uses rcu_assign_pointer() whch contains a memory barrier, so it's
impossible for another CPU to see a stale i_state.
inode = inode_insert5(new, hashval, test, set, data);
- if (unlikely(inode != new))
+ if (unlikely(inode != new)) {
+ spin_lock(&new->i_lock);
+ new->i_state |= I_FREEING;
+ spin_unlock(&new->i_lock);
+ inode_wait_for_writeback(new);
destroy_inode(new);
This doesn't make sense either. If an inode is returned here which is not
'new', then adding 'new' to the hash failed, and new was never visible
to another CPU.
@@ -1218,6 +1225,11 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino)
* allocated.
*/
spin_unlock(&inode_hash_lock);
+
+ spin_lock(&inode->i_lock);
+ inode->i_state |= I_FREEING;
+ spin_unlock(&inode->i_lock);
+ inode_wait_for_writeback(inode);
destroy_inode(inode);
Again, this doesn't make sense. This is also a codepath which failed to
make 'inode' visible to any other thread.
I don't understand how this patch could fix anything.
.
Thanks for your review，the underlying filesystem is ext4,
ext4_alloc_inode doesn't
allocate a new vfs inode from slab, and I found the "new inode" was used
by another
thread in vmcore, in other words, the new inode should be a new one ,
but not.
Maybe it's not a filesystem problem, and fixing this problem in
iget_locked is not
a good way, I 'll try to find the root cause and fix it.
