[PATCH 0/2] linux-rpdfs: rmdir() and unlink()

[PATCH 0/2] linux-rpdfs: rmdir() and unlink()

[Valerie Aurora]

Implement rpdfs_rmdir() and rpdfs_unlink() by copying rpdfs_rename()
and removing the unnecessary bits.

Valerie Aurora (2):
  Add rpdfs_unlink()
  Add rpdfs_rmdir()

 fs/rpdfs/dir.c | 102 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 102 insertions(+)

-- 
2.49.0

[PATCH 1/2] Add rpdfs_unlink()

[Valerie Aurora]

Copied from rpdfs_rename() and the extra bits removed.

Signed-off-by: Valerie Aurora <val@versity.com>
---
 fs/rpdfs/dir.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/fs/rpdfs/dir.c b/fs/rpdfs/dir.c
index 115f879d8fb1..9b33453ad5f6 100644
--- a/fs/rpdfs/dir.c
+++ b/fs/rpdfs/dir.c
@@ -491,6 +491,56 @@ static struct dentry *rpdfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	return ERR_PTR(create_and_instantiate_new(idmap, dir, dentry, S_IFDIR | mode));
 }
 
+static int rpdfs_unlink(struct inode *dir, struct dentry *dentry)
+{
+	struct rpdfs_fs_info *rfi = RPDFS_INODE_FS(dir);
+	struct inode *inode = d_inode(dentry);
+	struct key_dent *kd = NULL;
+	DECLARE_RPDFS_TXN(txn);
+	int ret;
+
+	kd = alloc_key_dent(dentry, inode);
+	if (!kd) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* prepare all the blocks for in the txn */
+	do {
+		ret = rpdfs_inode_txn_prepare(rfi, &txn, dir, RBAF_WRITE) ?:
+		      rpdfs_inode_txn_prepare(rfi, &txn, inode, RBAF_WRITE) ?:
+		      prepare_remove_entry(rfi, &txn, dir, kd);
+	} while (rpdfs_txn_retry(rfi, &txn, &ret));
+	if (ret < 0)
+		goto out;
+
+	/* apply changes to block structures */
+	apply_remove_entry(rfi, &txn, dir, kd);
+
+	/* update vfs inodes: first dir sizes and times */
+	i_size_write(dir, i_size_read(dir) - dentry->d_name.len);
+	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
+
+	/* now inode nlink and times */
+	if (inode->i_nlink == 0)
+		pr_warn("deleting inode %.*s with link count already 0",
+			dentry->d_name.len, dentry->d_name.name);
+	else
+		drop_nlink(inode);
+	inode_set_ctime_current(inode);
+
+	/* update block storage of vfs inodes */
+	rpdfs_inode_txn_update(rfi, &txn, dir);
+	rpdfs_inode_txn_update(rfi, &txn, inode);
+
+	ret = 0;
+out:
+	rpdfs_txn_reset(rfi, &txn);
+	kfree(kd);
+
+	return ret;
+}
+
 /*
  * The vfs has verified the cached directories.  Our inode refresh will
  * fail if the inodes have been reused, so we don't have to test if
@@ -680,6 +730,7 @@ const struct inode_operations rpdfs_dir_iops = {
 	.lookup		= rpdfs_lookup,
 	.mkdir		= rpdfs_mkdir,
 	.rename		= rpdfs_rename,
+	.unlink 	= rpdfs_unlink,
 };
 
 const struct file_operations rpdfs_dir_fops = {
-- 
2.49.0

[PATCH 2/2] Add rpdfs_rmdir()

[Valerie Aurora]

Copied from rpdfs_rename() and the extra bits removed.

Signed-off-by: Valerie Aurora <val@versity.com>
---
 fs/rpdfs/dir.c | 51 ++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/fs/rpdfs/dir.c b/fs/rpdfs/dir.c
index 9b33453ad5f6..cff995a7618c 100644
--- a/fs/rpdfs/dir.c
+++ b/fs/rpdfs/dir.c
@@ -541,6 +541,56 @@ static int rpdfs_unlink(struct inode *dir, struct dentry *dentry)
 	return ret;
 }
 
+static int rpdfs_rmdir(struct inode *dir, struct dentry *dentry)
+{
+	struct rpdfs_fs_info *rfi = RPDFS_INODE_FS(dir);
+	struct inode *inode = d_inode(dentry);
+	struct key_dent *kd = NULL;
+	DECLARE_RPDFS_TXN(txn);
+	int ret;
+
+	kd = alloc_key_dent(dentry, inode);
+	if (!kd) {
+		ret = -ENOMEM;
+		goto out;
+	}
+
+	/* prepare all the blocks in the txn */
+	do {
+		ret = rpdfs_inode_txn_prepare(rfi, &txn, dir, RBAF_WRITE) ?:
+		      rpdfs_inode_txn_prepare(rfi, &txn, inode, RBAF_WRITE) ?:
+		      prepare_remove_entry(rfi, &txn, dir, kd);
+	} while (rpdfs_txn_retry(rfi, &txn, &ret));
+	if (ret < 0)
+		goto out;
+
+	/* apply changes to block structures */
+	apply_remove_entry(rfi, &txn, dir, kd);
+
+	/* update vfs inodes: first dir sizes and times */
+	i_size_write(dir, i_size_read(dir) - dentry->d_name.len);
+	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
+
+	/* now inode nlink and times */
+	if (inode->i_nlink < 2)
+		pr_warn("deleting dir %.*s with link count %d < 2",
+			dentry->d_name.len, dentry->d_name.name, inode->i_nlink);
+	else
+		clear_nlink(inode);
+	inode_set_ctime_current(inode);
+
+	/* update block storage of vfs inodes */
+	rpdfs_inode_txn_update(rfi, &txn, dir);
+	rpdfs_inode_txn_update(rfi, &txn, inode);
+
+	ret = 0;
+out:
+	rpdfs_txn_reset(rfi, &txn);
+	kfree(kd);
+
+	return ret;
+}
+
 /*
  * The vfs has verified the cached directories.  Our inode refresh will
  * fail if the inodes have been reused, so we don't have to test if
@@ -731,6 +781,7 @@ const struct inode_operations rpdfs_dir_iops = {
 	.mkdir		= rpdfs_mkdir,
 	.rename		= rpdfs_rename,
 	.unlink 	= rpdfs_unlink,
+	.rmdir 		= rpdfs_rmdir,
 };
 
 const struct file_operations rpdfs_dir_fops = {
-- 
2.49.0

Re: [PATCH 2/2] Add rpdfs_rmdir()

[Zach Brown]

On Tue, Feb 24, 2026 at 05:41:57PM +0100, Valerie Aurora wrote:
> Copied from rpdfs_rename() and the extra bits removed.

...

> +	/* update vfs inodes: first dir sizes and times */
> +	i_size_write(dir, i_size_read(dir) - dentry->d_name.len);
> +	inode_set_mtime_to_ts(dir, inode_set_ctime_current(dir));
> +
> +	/* now inode nlink and times */
> +	if (inode->i_nlink < 2)
> +		pr_warn("deleting dir %.*s with link count %d < 2",
> +			dentry->d_name.len, dentry->d_name.name, inode->i_nlink);

I'm not sure how I feel about this warning.  I could see just not
bothering to warn about inconsistency, like we did up there with the
i_size_write() changing a possibly bad existing size.  If we warned we'd
probably want to show the inode number of the dir and inode?  Otherwise
it doesn't seem particularly helpful.

(Eventually we'll probably want a warning wrapper in pr.h that shows some
indication of which fs as well, like the sb_id for debugging.  But
nothing yet.)

Definitely don't print names on the console, though.  They can be
sensitive or absolute nonsense.  255 ascii bells :).

But more importantly..

> +	else
> +		clear_nlink(inode);

_unlink() and _rmdir() are identical, except for the magic silly decrease of
nlink on behalf of . and .. when the inode is a dir.  It looks like this
gets the inode nlink dec right, but misses the dec of the parent nlink.
Oh, and I guess testing that the dir is empty or returning ENOTEMPTY.

Instead of having two copies, just have unlink use S_ISDIR for the dir
differences.  Then the .rmdir and .unlink ops can both point to unlink.

- z