On 2018/3/1 5:31, Jason Gunthorpe wrote:
> On Mon, Feb 26, 2018 at 04:40:41PM +0800, Yixian Liu wrote:
>> diff --git a/providers/hns/hns_roce_u.h b/providers/hns/hns_roce_u.h
>> index 0291246..8e9634a 100644
>> +++ b/providers/hns/hns_roce_u.h
>> @@ -39,6 +39,7 @@
>> #include <infiniband/driver.h>
>> #include <util/udma_barrier.h>
>> #include <infiniband/verbs.h>
>> +#include <ccan/bitmap.h>
>> #include <ccan/container_of.h>
>>
>> #define HNS_ROCE_CQE_ENTRY_SIZE 0x20
>> @@ -93,6 +94,24 @@ struct hns_roce_buf {
>> unsigned int length;
>> };
>>
>> +/* the sw db length, on behalf of the qp/cq/srq length from left to right; */
>> +static const unsigned int db_size[] = {4, 4};
>
> no static data in header files
>
> This should also be something like
>
> static const unsigned int db_size[] = {
> [HNS_ROCE_QP_TYPE_DB] = 4,
> [HNS_ROCE_CQ_TYPE_DB= = 4,
> };
>
> For clarity
>
>> +/* the sw doorbell type; */
>> +enum hns_roce_db_type {
>> + HNS_ROCE_QP_TYPE_DB,
>> + HNS_ROCE_CQ_TYPE_DB,
>> + HNS_ROCE_DB_TYPE_NUM
>> +};
>> +
>> +struct hns_roce_db_page {
>> + struct hns_roce_db_page *prev, *next;
>> + struct hns_roce_buf buf;
>> + unsigned int num_db;
>> + unsigned int use_cnt;
>> + bitmap *bitmap;
>> +};
>> +
>> struct hns_roce_context {
>> struct verbs_context ibv_ctx;
>> void *uar;
>> @@ -110,6 +129,10 @@ struct hns_roce_context {
>> int num_qps;
>> int qp_table_shift;
>> int qp_table_mask;
>> +
>> + struct hns_roce_db_page *db_list[HNS_ROCE_DB_TYPE_NUM];
>> + pthread_mutex_t db_list_mutex;
>> +
>> unsigned int max_qp_wr;
>> unsigned int max_sge;
>> int max_cqe;
>> @@ -188,12 +211,14 @@ struct hns_roce_qp {
>> unsigned int sq_signal_bits;
>> struct hns_roce_wq sq;
>> struct hns_roce_wq rq;
>> + unsigned int *rdb;
>
> Sure looks like this should be a uint32_t * ?
>
>> +void hns_roce_free_db(struct hns_roce_context *ctx, unsigned int *db,
>> + enum hns_roce_db_type type)
>> +{
>> + struct hns_roce_db_page *page;
>> + uint32_t npos;
>> + uint32_t page_size;
>> +
>> + pthread_mutex_lock((pthread_mutex_t *)&ctx->db_list_mutex);
>> +
>> + page_size = to_hr_dev(ctx->ibv_ctx.context.device)->page_size;
>> + for (page = ctx->db_list[type]; page != NULL; page = page->next)
>> + if (((uintptr_t)db & (~((uintptr_t)page_size - 1))) ==
>> + (uintptr_t)(page->buf.buf))
>> + goto found;
>> +
>> + fprintf(stderr, "db page can't be found!\n");
>> + goto out;
>> +
>> +found:
>> + --page->use_cnt;
>> + if (!page->use_cnt) {
>> + if (page->prev)
>> + page->prev->next = page->next;
>> + else
>> + ctx->db_list[type] = page->next;
>> +
>> + if (page->next)
>> + page->next->prev = page->prev;
>> +
>> + hns_roce_clear_db_page(page);
>> + free(page);
>> + page = NULL;
>
> Don't needlessly NULL things.
>
>> + if ((to_hr_dev(pd->context->device)->hw_version != HNS_ROCE_HW_VER1) &&
>> + attr->cap.max_recv_sge) {
>> + qp->rdb = hns_roce_alloc_db(context, HNS_ROCE_QP_TYPE_DB);
>> + if (!qp->rdb) {
>> + fprintf(stderr, "alloc rdb buffer failed!\n");
>
> No prints from drivers (all places).
>
> Otherwise looks OK
>
> Jason
>
Thanks a lot, I will fix all the problems above.
--
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
