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

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