Re: [PATCH rdma-next v4 15/16] RDMA/mlx5: Use UMEM attribute for CQ doorbell record

[Jiri Pirko <jiri@resnulli.us>]

Tue, May 12, 2026 at 09:21:34PM +0200, jgg@ziepe.ca wrote:
>On Thu, May 07, 2026 at 02:52:30PM +0200, Jiri Pirko wrote:
>> +int mlx5_ib_db_map_user(struct mlx5_ib_ucontext *context,
>> +			struct ib_udata *udata, u16 attr_id,
>> +			unsigned long virt, struct mlx5_db *db)
>>  {
>> -	struct mlx5_ib_user_db_page *page;
>> +	struct mlx5_ib_user_db_page *page = NULL;
>> +	unsigned long dma_offset;
>>  	int err = 0;
>>  
>> +	if (udata) {
>> +		struct ib_umem *umem;
>> +
>> +		umem = ib_umem_get_attr(context->ibucontext.device, udata,
>> +					attr_id, sizeof(__be32) * 2, 0);
>> +		if (IS_ERR(umem))
>> +			return PTR_ERR(umem);
>> +		if (umem) {
>
>More IS_ERR_OR_NULL stuff..
>
>> +			/*
>> +			 * The 8-byte DBR is programmed to the device as one
>> +			 * DMA address, so it must stay within a single page.
>> +			 * An 8-byte range that crosses a page boundary may
>> +			 * be split across two non-contiguous DMA mappings.
>> +			 */
>> +			if (ib_umem_offset(umem) >
>> +			    PAGE_SIZE - sizeof(__be32) * 2) {
>> +				ib_umem_release(umem);
>> +				return -EINVAL;
>> +			}
>
>I think this can just be ib_umem_is_contiguous()

Right.

>
>> +			page = kzalloc_obj(*page);
>> +			if (!page) {
>> +				ib_umem_release(umem);
>> +				return -ENOMEM;
>> +			}
>> +			page->umem = umem;
>> +			dma_offset = ib_umem_offset(umem);
>> +		}
>> +	}
>> +
>>  	mutex_lock(&context->db_page_mutex);
>>  
>> +	if (page)
>> +		goto add_page;
>> +
>> +	dma_offset = virt & ~PAGE_MASK;
>> +
>>  	list_for_each_entry(page, &context->db_page_list, list)
>>  		if ((current->mm == page->mm) &&
>>  		    (page->user_virt == (virt & PAGE_MASK)))
>>  			goto found;
>
>Ah, this is why..
>
>I think this function should take in a ib_uverbs_buffer_desc and that
>should be stored inside the page instead of using virt. Then the
>functions flow doesn't really change, it searchs the page_list for a
>matching desc, otherwise it converts the desc to a umem and creates a
>new page. Refuse to match FD based descs
>
>That sort of suggests you want to split up the earlier patch so there
>is a seperate re-usable get desc function instead of bundling into
>ib_umem_get..

Okay, done that. Thanks!


>
>
>Jason