On Mon, Feb 26, 2018 at 03:43:09PM +0800, Liuyixian (Eason) wrote:
>
>
> 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.
Don't forget to do the user space side too.
Seems like you should have some kind of name for this, but I don't
really know what it is for..
> >> + 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?
No.. I mean:
++db_page->refcount_t;
Should be:
refcount_inc(&db_page->refcount);
To prevent userspace from overflowing the refcount and causing
something bad from happening.
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
