Commit d05dcfdf authored by Eric Van Hensbergen's avatar Eric Van Hensbergen

fs/9p: mitigate inode collisions

Detect and mitigate inode collsions that now occur since we
fixed 9p generating duplicate inode structures.  Underlying
cause of these appears to be a race condition between reuse
of inode numbers in underlying file system and cleanup of
inode numbers in the client.  Enabling caching
makes this much more likely to happen as it increases cleanup
latency due to writebacks.
Reported-by: default avatarKent Overstreet <kent.overstreet@linux.dev>
Signed-off-by: default avatarEric Van Hensbergen <ericvh@kernel.org>
parent ed30a4a5
...@@ -179,13 +179,14 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap, ...@@ -179,13 +179,14 @@ extern int v9fs_vfs_rename(struct mnt_idmap *idmap,
struct inode *old_dir, struct dentry *old_dentry, struct inode *old_dir, struct dentry *old_dentry,
struct inode *new_dir, struct dentry *new_dentry, struct inode *new_dir, struct dentry *new_dentry,
unsigned int flags); unsigned int flags);
extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid); extern struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid,
bool new);
extern const struct inode_operations v9fs_dir_inode_operations_dotl; extern const struct inode_operations v9fs_dir_inode_operations_dotl;
extern const struct inode_operations v9fs_file_inode_operations_dotl; extern const struct inode_operations v9fs_file_inode_operations_dotl;
extern const struct inode_operations v9fs_symlink_inode_operations_dotl; extern const struct inode_operations v9fs_symlink_inode_operations_dotl;
extern const struct netfs_request_ops v9fs_req_ops; extern const struct netfs_request_ops v9fs_req_ops;
extern struct inode *v9fs_fid_iget_dotl(struct super_block *sb, extern struct inode *v9fs_fid_iget_dotl(struct super_block *sb,
struct p9_fid *fid); struct p9_fid *fid, bool new);
/* other default globals */ /* other default globals */
#define V9FS_PORT 564 #define V9FS_PORT 564
...@@ -224,12 +225,12 @@ static inline int v9fs_proto_dotl(struct v9fs_session_info *v9ses) ...@@ -224,12 +225,12 @@ static inline int v9fs_proto_dotl(struct v9fs_session_info *v9ses)
*/ */
static inline struct inode * static inline struct inode *
v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid, v9fs_get_inode_from_fid(struct v9fs_session_info *v9ses, struct p9_fid *fid,
struct super_block *sb) struct super_block *sb, bool new)
{ {
if (v9fs_proto_dotl(v9ses)) if (v9fs_proto_dotl(v9ses))
return v9fs_fid_iget_dotl(sb, fid); return v9fs_fid_iget_dotl(sb, fid, new);
else else
return v9fs_fid_iget(sb, fid); return v9fs_fid_iget(sb, fid, new);
} }
#endif #endif
...@@ -364,7 +364,8 @@ void v9fs_evict_inode(struct inode *inode) ...@@ -364,7 +364,8 @@ void v9fs_evict_inode(struct inode *inode)
clear_inode(inode); clear_inode(inode);
} }
struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid) struct inode *
v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid, bool new)
{ {
dev_t rdev; dev_t rdev;
int retval; int retval;
...@@ -376,8 +377,18 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid) ...@@ -376,8 +377,18 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
inode = iget_locked(sb, QID2INO(&fid->qid)); inode = iget_locked(sb, QID2INO(&fid->qid));
if (unlikely(!inode)) if (unlikely(!inode))
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW)) if (!(inode->i_state & I_NEW)) {
return inode; if (!new) {
goto done;
} else {
p9_debug(P9_DEBUG_VFS, "WARNING: Inode collision %ld\n",
inode->i_ino);
iput(inode);
remove_inode_hash(inode);
inode = iget_locked(sb, QID2INO(&fid->qid));
WARN_ON(!(inode->i_state & I_NEW));
}
}
/* /*
* initialize the inode with the stat info * initialize the inode with the stat info
...@@ -401,11 +412,11 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid) ...@@ -401,11 +412,11 @@ struct inode *v9fs_fid_iget(struct super_block *sb, struct p9_fid *fid)
v9fs_set_netfs_context(inode); v9fs_set_netfs_context(inode);
v9fs_cache_inode_get_cookie(inode); v9fs_cache_inode_get_cookie(inode);
unlock_new_inode(inode); unlock_new_inode(inode);
done:
return inode; return inode;
error: error:
iget_failed(inode); iget_failed(inode);
return ERR_PTR(retval); return ERR_PTR(retval);
} }
/** /**
...@@ -437,8 +448,15 @@ static int v9fs_at_to_dotl_flags(int flags) ...@@ -437,8 +448,15 @@ static int v9fs_at_to_dotl_flags(int flags)
*/ */
static void v9fs_dec_count(struct inode *inode) static void v9fs_dec_count(struct inode *inode)
{ {
if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2) if (!S_ISDIR(inode->i_mode) || inode->i_nlink > 2) {
if (inode->i_nlink) {
drop_nlink(inode); drop_nlink(inode);
} else {
p9_debug(P9_DEBUG_VFS,
"WARNING: unexpected i_nlink zero %d inode %ld\n",
inode->i_nlink, inode->i_ino);
}
}
} }
/** /**
...@@ -489,6 +507,9 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags) ...@@ -489,6 +507,9 @@ static int v9fs_remove(struct inode *dir, struct dentry *dentry, int flags)
} else } else
v9fs_dec_count(inode); v9fs_dec_count(inode);
if (inode->i_nlink <= 0) /* no more refs unhash it */
remove_inode_hash(inode);
v9fs_invalidate_inode_attr(inode); v9fs_invalidate_inode_attr(inode);
v9fs_invalidate_inode_attr(dir); v9fs_invalidate_inode_attr(dir);
...@@ -554,7 +575,7 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir, ...@@ -554,7 +575,7 @@ v9fs_create(struct v9fs_session_info *v9ses, struct inode *dir,
/* /*
* instantiate inode and assign the unopened fid to the dentry * instantiate inode and assign the unopened fid to the dentry
*/ */
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb); inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, true);
if (IS_ERR(inode)) { if (IS_ERR(inode)) {
err = PTR_ERR(inode); err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, p9_debug(P9_DEBUG_VFS,
...@@ -683,7 +704,7 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry, ...@@ -683,7 +704,7 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
else if (IS_ERR(fid)) else if (IS_ERR(fid))
inode = ERR_CAST(fid); inode = ERR_CAST(fid);
else else
inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb); inode = v9fs_get_inode_from_fid(v9ses, fid, dir->i_sb, false);
/* /*
* If we had a rename on the server and a parallel lookup * If we had a rename on the server and a parallel lookup
* for the new name, then make sure we instantiate with * for the new name, then make sure we instantiate with
......
...@@ -52,7 +52,10 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode) ...@@ -52,7 +52,10 @@ static kgid_t v9fs_get_fsgid_for_create(struct inode *dir_inode)
return current_fsgid(); return current_fsgid();
} }
struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
struct inode *
v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid, bool new)
{ {
int retval; int retval;
struct inode *inode; struct inode *inode;
...@@ -62,8 +65,18 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid) ...@@ -62,8 +65,18 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
inode = iget_locked(sb, QID2INO(&fid->qid)); inode = iget_locked(sb, QID2INO(&fid->qid));
if (unlikely(!inode)) if (unlikely(!inode))
return ERR_PTR(-ENOMEM); return ERR_PTR(-ENOMEM);
if (!(inode->i_state & I_NEW)) if (!(inode->i_state & I_NEW)) {
return inode; if (!new) {
goto done;
} else { /* deal with race condition in inode number reuse */
p9_debug(P9_DEBUG_ERROR, "WARNING: Inode collision %lx\n",
inode->i_ino);
iput(inode);
remove_inode_hash(inode);
inode = iget_locked(sb, QID2INO(&fid->qid));
WARN_ON(!(inode->i_state & I_NEW));
}
}
/* /*
* initialize the inode with the stat info * initialize the inode with the stat info
...@@ -90,12 +103,11 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid) ...@@ -90,12 +103,11 @@ struct inode *v9fs_fid_iget_dotl(struct super_block *sb, struct p9_fid *fid)
goto error; goto error;
unlock_new_inode(inode); unlock_new_inode(inode);
done:
return inode; return inode;
error: error:
iget_failed(inode); iget_failed(inode);
return ERR_PTR(retval); return ERR_PTR(retval);
} }
struct dotl_openflag_map { struct dotl_openflag_map {
...@@ -247,7 +259,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry, ...@@ -247,7 +259,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err); p9_debug(P9_DEBUG_VFS, "p9_client_walk failed %d\n", err);
goto out; goto out;
} }
inode = v9fs_fid_iget_dotl(dir->i_sb, fid); inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
if (IS_ERR(inode)) { if (IS_ERR(inode)) {
err = PTR_ERR(inode); err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err); p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", err);
...@@ -340,7 +352,7 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap, ...@@ -340,7 +352,7 @@ static int v9fs_vfs_mkdir_dotl(struct mnt_idmap *idmap,
} }
/* instantiate inode and assign the unopened fid to the dentry */ /* instantiate inode and assign the unopened fid to the dentry */
inode = v9fs_fid_iget_dotl(dir->i_sb, fid); inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
if (IS_ERR(inode)) { if (IS_ERR(inode)) {
err = PTR_ERR(inode); err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
...@@ -776,7 +788,7 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir, ...@@ -776,7 +788,7 @@ v9fs_vfs_mknod_dotl(struct mnt_idmap *idmap, struct inode *dir,
err); err);
goto error; goto error;
} }
inode = v9fs_fid_iget_dotl(dir->i_sb, fid); inode = v9fs_fid_iget_dotl(dir->i_sb, fid, true);
if (IS_ERR(inode)) { if (IS_ERR(inode)) {
err = PTR_ERR(inode); err = PTR_ERR(inode);
p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n", p9_debug(P9_DEBUG_VFS, "inode creation failed %d\n",
......
...@@ -139,7 +139,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags, ...@@ -139,7 +139,7 @@ static struct dentry *v9fs_mount(struct file_system_type *fs_type, int flags,
else else
sb->s_d_op = &v9fs_dentry_operations; sb->s_d_op = &v9fs_dentry_operations;
inode = v9fs_get_inode_from_fid(v9ses, fid, sb); inode = v9fs_get_inode_from_fid(v9ses, fid, sb, true);
if (IS_ERR(inode)) { if (IS_ERR(inode)) {
retval = PTR_ERR(inode); retval = PTR_ERR(inode);
goto release_sb; goto release_sb;
......
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