This makes it possible for the inode to "move along" to the new location
when a file under a inodeident=path 9pfs is moved, and it will be reused
on next access to the new location.
Modifying the ino_path of children when renaming a directory is currently
not handled. Renaming non-empty directories still work, but the children
won't have their the inodes be reused after renaming.
Inodes will also not be reused on server-side rename, since there is no
way for us to know about it. From our perspective this is
indistinguishable from a new file being created in the destination that
just happened to have the same qid, and the original file being deleted.
Signed-off-by: Tingmao Wang <m@xxxxxxxxxx>
Cc: "Mickaël Salaün" <mic@xxxxxxxxxxx>
Cc: "Günther Noack" <gnoack@xxxxxxxxxx>
---
New patch in v2
fs/9p/ino_path.c | 3 ++-
fs/9p/v9fs.h | 3 +++
fs/9p/vfs_inode.c | 30 ++++++++++++++++++++++++++++++
fs/9p/vfs_inode_dotl.c | 6 ++++++
4 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/fs/9p/ino_path.c b/fs/9p/ino_path.c
index a03145e08a9d..ee4752b9f796 100644
--- a/fs/9p/ino_path.c
+++ b/fs/9p/ino_path.c
@@ -27,7 +27,8 @@ struct v9fs_ino_path *make_ino_path(struct dentry *dentry)
struct dentry *curr = dentry;
ssize_t i;
- lockdep_assert_held_read(&v9fs_dentry2v9ses(dentry)->rename_sem);
+ /* Either read or write lock held is ok */
+ lockdep_assert_held(&v9fs_dentry2v9ses(dentry)->rename_sem);
might_sleep(); /* Allocation below might block */
rcu_read_lock();
diff --git a/fs/9p/v9fs.h b/fs/9p/v9fs.h
index bacd0052e22c..c441fa8e757b 100644
--- a/fs/9p/v9fs.h
+++ b/fs/9p/v9fs.h
@@ -157,6 +157,9 @@ struct v9fs_inode {
/*
* Stores the path of the file this inode is for, only for filesystems
* with inode_ident=path. Lifetime is the same as this inode.
+ * Read/write to this pointer should be under the target v9fs's
+ * rename_sem to protect against races (except when initializing or
+ * freeing an inode, at which point nobody else has reference to us)
*/
struct v9fs_ino_path *path;
};
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 4b4712eafe4d..68a1837ff3dc 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -532,6 +532,12 @@ static struct inode *v9fs_qid_iget(struct super_block *sb, struct p9_qid *qid,
v9inode = V9FS_I(inode);
if (dentry) {
+ /*
+ * In order to make_ino_path, we need at least a read lock on the
+ * rename_sem. Since we re initializing a new inode, there is no
+ * risk of races with another task trying to write to
+ * v9inode->path, so we do not need an actual down_write.
+ */
down_read(&v9ses->rename_sem);
v9inode->path = make_ino_path(dentry);
up_read(&v9ses->rename_sem);
@@ -983,18 +989,21 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
{
int retval;
struct inode *old_inode;
+ struct v9fs_inode *old_v9inode;
struct inode *new_inode;
struct v9fs_session_info *v9ses;
struct p9_fid *oldfid = NULL, *dfid = NULL;
struct p9_fid *olddirfid = NULL;
struct p9_fid *newdirfid = NULL;
struct p9_wstat wstat;
+ struct v9fs_ino_path *new_ino_path = NULL;
if (flags)
return -EINVAL;
p9_debug(P9_DEBUG_VFS, "\n");
old_inode = d_inode(old_dentry);
+ old_v9inode = V9FS_I(old_inode);
new_inode = d_inode(new_dentry);
v9ses = v9fs_inode2v9ses(old_inode);
oldfid = v9fs_fid_lookup(old_dentry);
@@ -1022,6 +1031,17 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
}
down_write(&v9ses->rename_sem);
+ if (v9fs_inode_ident_path(v9ses)) {
+ /*
+ * Try to allocate this first, and don't actually do rename if
+ * allocation fails.
+ */
+ new_ino_path = make_ino_path(new_dentry);
+ if (!new_ino_path) {
+ retval = -ENOMEM;
+ goto error_locked;
+ }
+ }
if (v9fs_proto_dotl(v9ses)) {
retval = p9_client_renameat(olddirfid, old_dentry->d_name.name,
newdirfid, new_dentry->d_name.name);
@@ -1061,6 +1081,15 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
v9fs_invalidate_inode_attr(old_inode);
v9fs_invalidate_inode_attr(old_dir);
v9fs_invalidate_inode_attr(new_dir);
+ if (v9fs_inode_ident_path(v9ses)) {
+ /*
+ * We currently have rename_sem write lock, which protects all
+ * v9inode->path in this fs.
+ */
+ free_ino_path(old_v9inode->path);
+ old_v9inode->path = new_ino_path;
+ new_ino_path = NULL;
+ }
/* successful rename */
d_move(old_dentry, new_dentry);
@@ -1068,6 +1097,7 @@ v9fs_vfs_rename(struct mnt_idmap *idmap, struct inode *old_dir,
up_write(&v9ses->rename_sem);
error:
+ free_ino_path(new_ino_path);
p9_fid_put(newdirfid);
p9_fid_put(olddirfid);
p9_fid_put(oldfid);
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index d008e82256ac..a3f70dd422fb 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -232,6 +232,12 @@ static struct inode *v9fs_qid_iget_dotl(struct super_block *sb,
v9inode = V9FS_I(inode);
if (dentry) {
+ /*
+ * In order to make_ino_path, we need at least a read lock on the
+ * rename_sem. Since we re initializing a new inode, there is no
+ * risk of races with another task trying to write to
+ * v9inode->path, so we do not need an actual down_write.
+ */
down_read(&v9ses->rename_sem);
v9inode->path = make_ino_path(dentry);
up_read(&v9ses->rename_sem);
--
2.51.0
