Re: [PATCH rdma-next v2 02/11] IB/core: Introduce FRMR pools

[Jason Gunthorpe]

On Mon, Dec 22, 2025 at 02:40:37PM +0200, Edward Srouji wrote:
> +static int compare_keys(struct ib_frmr_key *key1, struct ib_frmr_key *key2)
> +{
> +	int res;
> +
> +	res = key1->ats - key2->ats;
> +	if (res)
> +		return res;
> +
> +	res = key1->access_flags - key2->access_flags;
> +	if (res)
> +		return res;
> +
> +	res = key1->vendor_key - key2->vendor_key;
> +	if (res)
> +		return res;
> +
> +	res = key1->kernel_vendor_key - key2->kernel_vendor_key;
> +	if (res)
> +		return res;

This stuff should be using cmp_int().

> +static struct ib_frmr_pool *ib_frmr_pool_find(struct ib_frmr_pools *pools,
> +					      struct ib_frmr_key *key)
> +{
> +	struct rb_node *node = pools->rb_root.rb_node;
> +	struct ib_frmr_pool *pool;
> +	int cmp;
> +
> +	/* find operation is done under read lock for performance reasons.
> +	 * The case of threads failing to find the same pool and creating it
> +	 * is handled by the create_frmr_pool function.
> +	 */
> +	read_lock(&pools->rb_lock);
> +	while (node) {
> +		pool = rb_entry(node, struct ib_frmr_pool, node);
> +		cmp = compare_keys(&pool->key, key);
> +		if (cmp < 0) {
> +			node = node->rb_right;
> +		} else if (cmp > 0) {
> +			node = node->rb_left;
> +		} else {
> +			read_unlock(&pools->rb_lock);
> +			return pool;
> +		}

Use the rb_find() helper

> +static struct ib_frmr_pool *create_frmr_pool(struct ib_device *device,
> +					     struct ib_frmr_key *key)
> +{
> +	struct rb_node **new = &device->frmr_pools->rb_root.rb_node,
> +		       *parent = NULL;
> +	struct ib_frmr_pools *pools = device->frmr_pools;
> +	struct ib_frmr_pool *pool;
> +	int cmp;
> +
> +	pool = kzalloc(sizeof(*pool), GFP_KERNEL);
> +	if (!pool)
> +		return ERR_PTR(-ENOMEM);
> +
> +	memcpy(&pool->key, key, sizeof(*key));
> +	INIT_LIST_HEAD(&pool->queue.pages_list);
> +	spin_lock_init(&pool->lock);
> +
> +	write_lock(&pools->rb_lock);
> +	while (*new) {
> +		parent = *new;
> +		cmp = compare_keys(
> +			&rb_entry(parent, struct ib_frmr_pool, node)->key, key);
> +		if (cmp < 0)
> +			new = &((*new)->rb_left);
> +		else
> +			new = &((*new)->rb_right);
> +		/* If a different thread has already created the pool, return
> +		 * it. The insert operation is done under the write lock so we
> +		 * are sure that the pool is not inserted twice.
> +		 */
> +		if (cmp == 0) {
> +			write_unlock(&pools->rb_lock);
> +			kfree(pool);
> +			return rb_entry(parent, struct ib_frmr_pool, node);
> +		}
> +	}
> +
> +	rb_link_node(&pool->node, parent, new);

I think this is rb_find_add() ?

Jason