On 2018/2/16 7:23, Jason Gunthorpe wrote:
> On Mon, Feb 12, 2018 at 06:14:08PM +0800, Yixian Liu wrote:
>> +int hns_roce_db_map_user(struct hns_roce_ucontext *context, unsigned long virt,
>> + struct hns_roce_db *db)
>> +{
>> + struct hns_roce_user_db_page *db_page;
>> + int ret = 0;
>> +
>> + mutex_lock(&context->db_page_mutex);
>> +
>> + list_for_each_entry(db_page, &context->db_page_list, list)
>> + if (db_page->user_virt == (virt & PAGE_MASK))
>> + goto found;
>> +
>> + db_page = kmalloc(sizeof(*db_page), GFP_KERNEL);
>> + if (!db_page) {
>> + ret = -ENOMEM;
>> + goto out;
>> + }
>> +
>> + db_page->user_virt = (virt & PAGE_MASK);
>> + db_page->refcount_t = 0;
>> + db_page->umem = ib_umem_get(&context->ibucontext, virt & PAGE_MASK,
>> + PAGE_SIZE, 0, 0);
>
> It seems quite odd to call something 'doorbell' that exists in system
> memory? doorbell should exist in BAR memory..
>
I will rename db_page to page to avoid confusion in v5.
>> + if (IS_ERR(db_page->umem)) {
>> + ret = PTR_ERR(db_page->umem);
>> + kfree(db_page);
>> + goto out;
>> + }
>> +
>> + list_add(&db_page->list, &context->db_page_list);
>> +
>> +found:
>> + db->dma = sg_dma_address(db_page->umem->sg_head.sgl) +
>> + (virt & ~PAGE_MASK);
>> + db->u.user_page = db_page;
>> + ++db_page->refcount_t;
>
> Since user space controls this it should be a refcount_t
Do you mean I should pass the refcount_t from user space to kernel and
here use the user space refcount_t?
If it is, I think this revision is more or less unnecessary.
As kernel refcount_t will be changed only when mapping, it is more
reasonable to update this value by kernel.
>
> Jason
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
