[PATCH 16/23] Add ngnfs_dir_unlink(), ngnfs_dir_rmdir(), and debugfs commands

[Zach Brown]

On Fri, Apr 04, 2025 at 08:45:32PM +0200, Valerie Aurora wrote:
> Allow removal of links to files and directories. Also simplify calling
> convention for update_dir() and add ngnfs_inode_update() to change
> link counts for the target of a directory entry.


> +static int remove_dirent_wr(struct ngnfs_btree_key *key, void *val, size_t size, void *arg,
> +			    struct ngnfs_btree_op *op)
> +{
> +	struct ngnfs_dirent *dent = val;
> +	struct dirent_args *da = arg;
> +
> +	if (!dent)
> +		return -ENOENT;
> +
> +	if (!names_equal(dent->name, dent->name_len, da->dent.name, da->dent.name_len))
> +		return NGNFS_BTREE_ITER_CONTINUE;
> +
> +	/* ino says we found it, dent tells us how to update parent dir */
> +	da->ig.ino = le64_to_cpu(dent->ig.ino);
> +	da->ig.gen = le64_to_cpu(dent->ig.gen);
> +	memcpy(&da->dent, dent, size);

This is copying the ig twice.  Do we need to?  It'd be nice if we could
simplify it down to using the ig in the dent without the other copy in
the args.

> +static int do_unlink(struct ngnfs_fs_info *nfi, struct ngnfs_inode_ino_gen *dir_ig, char *name,
> +		     size_t name_len, int flags)
> +{
> +	struct {
> +		struct ngnfs_transaction txn;
> +		struct ngnfs_inode_txn_ref dir;
> +		struct ngnfs_inode_txn_ref target;
> +		struct dirent_args da;
> +		u64 ino;
> +		u64 nsec;
> +	} *op;
> +	int ret;
> +
> +	if (name_len > NGNFS_NAME_MAX) {
> +		ret = -ENAMETOOLONG;
> +		goto out;
> +	}
> +
> +	op = kmalloc(sizeof(*op), GFP_NOFS);
> +	if (!op) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	ngnfs_txn_init(&op->txn);
> +	op->nsec = ktime_to_ns(ktime_get_real());
> +	init_dirent_args(&op->da, name, name_len, 0);
> +
> +	do {
> +		ret = ngnfs_inode_get(nfi, &op->txn, NBF_WRITE, dir_ig, &op->dir)		?:
> +		      check_ifmt(op->dir.ninode, S_IFDIR, -ENOTDIR)				?:
> +		      remove_dirent(nfi, &op->txn, &op->dir, &op->da)				?:
> +		      ngnfs_inode_get(nfi, &op->txn, NBF_WRITE, &op->da.ig, &op->target)	?:
> +		      check_remove_dirent(nfi, &op->txn, &op->dir, &op->da, flags)		?:
> +		      ngnfs_inode_update(op->target.tblk, op->target.ninode, -1)		?:
> +		      update_dir(op->dir.tblk, op->dir.ninode, &op->da, -1);

This gets the target inode, and then check_remove_dirent gets its own
copy of the target inode again to see if it's an empty dir.  Checking
should use the inode that we got here.

> +/*
> + * Update an inode to reflect the addition or removal of one or more links to it.
> + */
> +int ngnfs_inode_update(struct ngnfs_txn_block *tblk, struct ngnfs_inode *inode, s32 delta)
> +{
> +	s32 nlink = le32_to_cpu(inode->nlink);
> +
> +	if ((delta > 0) && (nlink > NGNFS_LINK_MAX - delta))
> +		return -EMLINK;
> +
> +	/* nlink < 0 is a bug */
> +	BUG_ON((delta < 0) && (nlink + delta < 0));

I'm not going to get too exercised about this sort of thing now.  We'll
have to do a cleanup pass that removes these regardless, and a few more
here or there isn't a big deal.

But in general, when reasonable, let's try to return errors in the face
of corruption.  Repair will be responsible for cleaning up these kinds
of inconsistencies.  (Imagine a notification pipeline from helpers that
raise these errors to repair going after them.)

- z