Re: [PATCH 6/8] rpdfs: add file data allocation and lookup routines

Zach Brown <[email protected]> Mon, 27 Apr 2026 16:02:01 -0700
Newsgroups dev.linux.lists.rpdfs-devel
Message-ID <[email protected]>
On Fri, Apr 24, 2026 at 04:05:18PM +0200, Valerie Aurora wrote:
> Add routines to lookup and allocate file data and mapping blocks.

> +static int alloc_block_ref(struct rpdfs_fs_info *rfi, struct rpdfs_transaction *txn,
> +			   struct rpdfs_block_ref *ref, struct rpdfs_block_handle **blk_hnd)
> +{
> +	int ret;
> +
> +	ret = rpdfs_txn_acquire_alloc(rfi, txn, blk_hnd);
> +	if (ret < 0)
> +		goto out;
> +
> +	memset((*blk_hnd)->data, 0, RPDFS_BLOCK_SIZE);
> +
> +	ref->bnr = cpu_to_le64((*blk_hnd)->bnr);
> +	ref->alloc_counter = 0; /* XXX */
> +out:
> +	rpdfs_prd("ret %d bnr %llu", ret, ref->bnr);
> +	return ret;
> +}

This needs to set the place for the new blocks.  Maybe something like
_PLACE_DATA_MAP and _PLACE_DATA_BLOCK that are after the metadata place
definitions.  The place offset could be the lblk for the data blocks and
some expression of level and lblk for the mapping blocks.

> +static int grow_height(struct rpdfs_fs_info *rfi, struct rpdfs_transaction *txn,
> +		       struct inode *inode, struct rpdfs_block_handle *inode_hnd, int height)
> +{
> +	struct rpdfs_inode_info *ri = RPDFS_I(inode);
> +	struct rpdfs_block_handle *hnd = NULL;
> +	struct rpdfs_block_ref *ref;
> +	struct rpdfs_map_block *iblk;
> +	struct rpdfs_data_root *dr;
> +	struct rpdfs_block_ref orig_root;
> +	u8 level;
> +	int ret;
> +
> +	dr = &ri->data_root;
> +
> +	rpdfs_prd("dr->height %u goal height %u", dr->height, height);
> +
> +	ret = 0;
> +	if (height > dr->height) {
> +		/* start at root and fill in till we get to existing tree */
> +		level = height;
> +		orig_root = dr->ref; /* save current root of tree */
> +		ref = &dr->ref;
> +
> +		while (level-- > dr->height) {
> +			rpdfs_prd("allocing block at level %d", level + 1);
> +			hnd = NULL;
> +			ret = alloc_block_ref(rfi, txn, &dr->ref, &hnd);
> +			if (ret < 0)
> +				goto out;
> +			iblk = hnd->data;
> +			/* growing height will always index old data to 0 */
> +			ref = &iblk->refs[0];
> +			rpdfs_block_release(rfi, &hnd);
> +		}
> +		rpdfs_prd("grafting old height %d tree to new %d height tree",
> +			  dr->height, height);
> +		/* now put existing tree in mapping block */
> +		*ref = orig_root;
> +		/* and update the height in the data root in the inode */
> +		ri->data_root.height = height;
> +	}
> +out:
> +	return ret;

You can lose most of this by always operating on the root instead of
juggling local temporary variables.

	while (ri->data_root.height < height) {
		alloc(&ref);
		...
		iblk->refs[0] = ri->data_root->ref;
		ri->data_root->ref = ref;
		ri->data_root->height++;
	}

> + * XXX: Takes write access on map blocks on write even when it only
> + * needs to read them. Redescend with write when we know we have to
> + * allocate?

Yeah, we don't want to be dirtying parent mapping blocks when streaming
writes.  It shouldn't have to redescend, though, just release and
acquire the parent block.  It could save the ref to the parent.

- z