Commit 5a9b911b authored by Mateusz Guzik's avatar Mateusz Guzik Committed by Christian Brauner

vfs: partially sanitize i_state zeroing on inode creation

new_inode used to have the following:
	spin_lock(&inode_lock);
	inodes_stat.nr_inodes++;
	list_add(&inode->i_list, &inode_in_use);
	list_add(&inode->i_sb_list, &sb->s_inodes);
	inode->i_ino = ++last_ino;
	inode->i_state = 0;
	spin_unlock(&inode_lock);

over time things disappeared, got moved around or got replaced (global
inode lock with a per-inode lock), eventually this got reduced to:
	spin_lock(&inode->i_lock);
	inode->i_state = 0;
	spin_unlock(&inode->i_lock);

But the lock acquire here does not synchronize against anyone.

Additionally iget5_locked performs i_state = 0 assignment without any
locks to begin with, the two combined look confusing at best.

It looks like the current state is a leftover which was not cleaned up.

Ideally it would be an invariant that i_state == 0 to begin with, but
achieving that would require dealing with all filesystem alloc handlers
one by one.

In the meantime drop the misleading locking and move i_state zeroing to
inode_init_always so that others don't need to deal with it by hand.
Signed-off-by: default avatarMateusz Guzik <mjguzik@gmail.com>
Link: https://lore.kernel.org/r/20240611120626.513952-3-mjguzik@gmail.comReviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarChristian Brauner <brauner@kernel.org>
parent ddd4cd48
...@@ -162,6 +162,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) ...@@ -162,6 +162,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
inode->i_sb = sb; inode->i_sb = sb;
inode->i_blkbits = sb->s_blocksize_bits; inode->i_blkbits = sb->s_blocksize_bits;
inode->i_flags = 0; inode->i_flags = 0;
inode->i_state = 0;
atomic64_set(&inode->i_sequence, 0); atomic64_set(&inode->i_sequence, 0);
atomic_set(&inode->i_count, 1); atomic_set(&inode->i_count, 1);
inode->i_op = &empty_iops; inode->i_op = &empty_iops;
...@@ -231,6 +232,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode) ...@@ -231,6 +232,7 @@ int inode_init_always(struct super_block *sb, struct inode *inode)
if (unlikely(security_inode_alloc(inode))) if (unlikely(security_inode_alloc(inode)))
return -ENOMEM; return -ENOMEM;
this_cpu_inc(nr_inodes); this_cpu_inc(nr_inodes);
return 0; return 0;
...@@ -1023,14 +1025,7 @@ EXPORT_SYMBOL(get_next_ino); ...@@ -1023,14 +1025,7 @@ EXPORT_SYMBOL(get_next_ino);
*/ */
struct inode *new_inode_pseudo(struct super_block *sb) struct inode *new_inode_pseudo(struct super_block *sb)
{ {
struct inode *inode = alloc_inode(sb); return alloc_inode(sb);
if (inode) {
spin_lock(&inode->i_lock);
inode->i_state = 0;
spin_unlock(&inode->i_lock);
}
return inode;
} }
/** /**
...@@ -1254,7 +1249,6 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval, ...@@ -1254,7 +1249,6 @@ struct inode *iget5_locked(struct super_block *sb, unsigned long hashval,
struct inode *new = alloc_inode(sb); struct inode *new = alloc_inode(sb);
if (new) { if (new) {
new->i_state = 0;
inode = inode_insert5(new, hashval, test, set, data); inode = inode_insert5(new, hashval, test, set, data);
if (unlikely(inode != new)) if (unlikely(inode != new))
destroy_inode(new); destroy_inode(new);
...@@ -1297,7 +1291,6 @@ struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval, ...@@ -1297,7 +1291,6 @@ struct inode *iget5_locked_rcu(struct super_block *sb, unsigned long hashval,
new = alloc_inode(sb); new = alloc_inode(sb);
if (new) { if (new) {
new->i_state = 0;
inode = inode_insert5(new, hashval, test, set, data); inode = inode_insert5(new, hashval, test, set, data);
if (unlikely(inode != new)) if (unlikely(inode != new))
destroy_inode(new); destroy_inode(new);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment