All of lore.kernel.org
 help / color / mirror / Atom feed
From: cel@kernel.org
To: Christian Brauner <brauner@kernel.org>
Cc: <linux-fsdevel@vger.kernel.org>, Chuck Lever <chuck.lever@oracle.com>
Subject: [PATCH v1 1/2] shmem: Fix shmem_rename2()
Date: Thu, 11 Apr 2024 14:26:10 -0400	[thread overview]
Message-ID: <20240411182611.203328-2-cel@kernel.org> (raw)
In-Reply-To: <20240411182611.203328-1-cel@kernel.org>

From: Chuck Lever <chuck.lever@oracle.com>

When renaming onto an existing directory entry, user space expects
the replacement entry to have the same directory offset as the
original one.

The details of handling directory offsets during a rename are moved
to fs/libfs.c so that they can be reused by other API consumers.

For backporting to stable kernels: use xa_store() rather than
mtree_store(), and octx->xa rather than octx->mt. See commit
0e4a862174f2 ("libfs: Convert simple directory offsets to use a
Maple Tree") for details.

Link: https://gitlab.alpinelinux.org/alpine/aports/-/issues/15966
Fixes: a2e459555c5f ("shmem: stable directory offsets")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/libfs.c         | 53 ++++++++++++++++++++++++++++++++++++++++------
 include/linux/fs.h |  2 ++
 mm/shmem.c         |  3 +--
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 3a6f2cb364f8..ff767288e5dd 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -295,6 +295,17 @@ int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry)
 	return 0;
 }
 
+/*
+ * Internal helper for use when it is known that the tree entry at
+ * @index is already NULL.
+ */
+static int simple_offset_store(struct offset_ctx *octx, struct dentry *dentry,
+			       long index)
+{
+	offset_set(dentry, index);
+	return mtree_store(&octx->mt, index, dentry, GFP_KERNEL);
+}
+
 /**
  * simple_offset_remove - Remove an entry to a directory's offset map
  * @octx: directory offset ctx to be updated
@@ -345,6 +356,35 @@ int simple_offset_empty(struct dentry *dentry)
 	return ret;
 }
 
+/**
+ * simple_offset_rename - handle directory offsets for rename
+ * @old_dir: parent directory of source entry
+ * @old_dentry: dentry of source entry
+ * @new_dir: parent_directory of destination entry
+ * @new_dentry: dentry of destination
+ *
+ * Caller provides appropriate serialization.
+ *
+ * Returns zero on success, a negative errno value on failure.
+ */
+int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
+			 struct inode *new_dir, struct dentry *new_dentry)
+{
+	struct offset_ctx *old_ctx = old_dir->i_op->get_offset_ctx(old_dir);
+	struct offset_ctx *new_ctx = new_dir->i_op->get_offset_ctx(new_dir);
+	long new_index = dentry2offset(new_dentry);
+
+	simple_offset_remove(old_ctx, old_dentry);
+
+	/*
+	 * When the destination entry already exists, user space expects
+	 * its directory offset value to be unchanged after the rename.
+	 */
+	if (new_index)
+		return simple_offset_store(new_ctx, old_dentry, new_index);
+	return simple_offset_add(new_ctx, old_dentry);
+}
+
 /**
  * simple_offset_rename_exchange - exchange rename with directory offsets
  * @old_dir: parent of dentry being moved
@@ -352,6 +392,9 @@ int simple_offset_empty(struct dentry *dentry)
  * @new_dir: destination parent
  * @new_dentry: destination dentry
  *
+ * This API preserves the directory offset values. Caller provides
+ * appropriate serialization.
+ *
  * Returns zero on success. Otherwise a negative errno is returned and the
  * rename is rolled back.
  */
@@ -369,11 +412,11 @@ int simple_offset_rename_exchange(struct inode *old_dir,
 	simple_offset_remove(old_ctx, old_dentry);
 	simple_offset_remove(new_ctx, new_dentry);
 
-	ret = simple_offset_add(new_ctx, old_dentry);
+	ret = simple_offset_store(new_ctx, old_dentry, new_index);
 	if (ret)
 		goto out_restore;
 
-	ret = simple_offset_add(old_ctx, new_dentry);
+	ret = simple_offset_store(old_ctx, new_dentry, old_index);
 	if (ret) {
 		simple_offset_remove(new_ctx, old_dentry);
 		goto out_restore;
@@ -388,10 +431,8 @@ int simple_offset_rename_exchange(struct inode *old_dir,
 	return 0;
 
 out_restore:
-	offset_set(old_dentry, old_index);
-	mtree_store(&old_ctx->mt, old_index, old_dentry, GFP_KERNEL);
-	offset_set(new_dentry, new_index);
-	mtree_store(&new_ctx->mt, new_index, new_dentry, GFP_KERNEL);
+	(void)simple_offset_store(old_ctx, old_dentry, old_index);
+	(void)simple_offset_store(new_ctx, new_dentry, new_index);
 	return ret;
 }
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8dfd53b52744..b09f14132110 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3340,6 +3340,8 @@ void simple_offset_init(struct offset_ctx *octx);
 int simple_offset_add(struct offset_ctx *octx, struct dentry *dentry);
 void simple_offset_remove(struct offset_ctx *octx, struct dentry *dentry);
 int simple_offset_empty(struct dentry *dentry);
+int simple_offset_rename(struct inode *old_dir, struct dentry *old_dentry,
+			 struct inode *new_dir, struct dentry *new_dentry);
 int simple_offset_rename_exchange(struct inode *old_dir,
 				  struct dentry *old_dentry,
 				  struct inode *new_dir,
diff --git a/mm/shmem.c b/mm/shmem.c
index 0aad0d9a621b..c0fb65223963 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -3473,8 +3473,7 @@ static int shmem_rename2(struct mnt_idmap *idmap,
 			return error;
 	}
 
-	simple_offset_remove(shmem_get_offset_ctx(old_dir), old_dentry);
-	error = simple_offset_add(shmem_get_offset_ctx(new_dir), old_dentry);
+	error = simple_offset_rename(old_dir, old_dentry, new_dir, new_dentry);
 	if (error)
 		return error;
 
-- 
2.44.0


  reply	other threads:[~2024-04-11 18:26 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-11 18:26 [PATCH v1 0/2] Fix shmem_rename2 directory offset calculation cel
2024-04-11 18:26 ` cel [this message]
2024-04-11 18:26 ` [PATCH v1 2/2] libfs: Clean up the simple_offset API cel
2024-04-12  5:50   ` kernel test robot
2024-04-12 14:29 ` [PATCH v1 0/2] Fix shmem_rename2 directory offset calculation Chuck Lever
2024-04-15 14:48   ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240411182611.203328-2-cel@kernel.org \
    --to=cel@kernel.org \
    --cc=brauner@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.