Merge pull request #1450 from hginjgerx/bug

libhns: Bugfixes and cleanups

providers/hns/hns_roce_u.c

@@ -145,6 +145,47 @@ static int set_context_attr(struct hns_roce_device *hr_dev,
  return 0;
 }
 
+static int hns_roce_init_context_lock(struct hns_roce_context *context)
+{
+ int ret;
+
+ ret = pthread_spin_init(&context->uar_lock, PTHREAD_PROCESS_PRIVATE);
+ if (ret)
+ return ret;
+
+ ret = pthread_mutex_init(&context->qp_table_mutex, NULL);
+ if (ret)
+ goto destroy_uar_lock;
+
+ ret = pthread_mutex_init(&context->srq_table_mutex, NULL);
+ if (ret)
+ goto destroy_qp_mutex;
+
+ ret = pthread_mutex_init(&context->db_list_mutex, NULL);
+ if (ret)
+ goto destroy_srq_mutex;
+
+ return 0;
+
+destroy_srq_mutex:
+ pthread_mutex_destroy(&context->srq_table_mutex);
+
+destroy_qp_mutex:
+ pthread_mutex_destroy(&context->qp_table_mutex);
+
+destroy_uar_lock:
+ pthread_spin_destroy(&context->uar_lock);
+ return ret;
+}
+
+static void hns_roce_destroy_context_lock(struct hns_roce_context *context)
+{
+ pthread_spin_destroy(&context->uar_lock);
+ pthread_mutex_destroy(&context->qp_table_mutex);
+ pthread_mutex_destroy(&context->srq_table_mutex);
+ pthread_mutex_destroy(&context->db_list_mutex);
+}
+
 static struct verbs_context *hns_roce_alloc_context(struct ibv_device *ibdev,
  int cmd_fd,
  void *private_data)
@@ -163,26 +204,28 @@ static struct verbs_context *hns_roce_alloc_context(struct ibv_device *ibdev,
  HNS_ROCE_CQE_INLINE_FLAGS;
  if (ibv_cmd_get_context(&context->ibv_ctx, &cmd.ibv_cmd, sizeof(cmd),
  &resp.ibv_resp, sizeof(resp)))
- goto err_free;
+ goto err_ibv_cmd;
+
+ if (hns_roce_init_context_lock(context))
+ goto err_ibv_cmd;
 
  if (set_context_attr(hr_dev, context, &resp))
- goto err_free;
+ goto err_set_attr;
 
  context->uar = mmap(NULL, hr_dev->page_size, PROT_READ | PROT_WRITE,
  MAP_SHARED, cmd_fd, 0);
  if (context->uar == MAP_FAILED)
- goto err_free;
+ goto err_set_attr;
 
- pthread_mutex_init(&context->qp_table_mutex, NULL);
- pthread_mutex_init(&context->srq_table_mutex, NULL);
- pthread_spin_init(&context->uar_lock, PTHREAD_PROCESS_PRIVATE);
 
  verbs_set_ops(&context->ibv_ctx, &hns_common_ops);
  verbs_set_ops(&context->ibv_ctx, &hr_dev->u_hw->hw_ops);
 
  return &context->ibv_ctx;
 
-err_free:
+err_set_attr:
+ hns_roce_destroy_context_lock(context);
+err_ibv_cmd:
  verbs_uninit_context(&context->ibv_ctx);
  free(context);
  return NULL;
@@ -194,6 +237,7 @@ static void hns_roce_free_context(struct ibv_context *ibctx)
  struct hns_roce_context *context = to_hr_ctx(ibctx);
 
  munmap(context->uar, hr_dev->page_size);
+ hns_roce_destroy_context_lock(context);
  verbs_uninit_context(&context->ibv_ctx);
  free(context);
 }

providers/hns/hns_roce_u_db.c

@@ -116,6 +116,8 @@ void *hns_roce_alloc_db(struct hns_roce_context *ctx,
 
 out:
  pthread_mutex_unlock((pthread_mutex_t *)&ctx->db_list_mutex);
+ if (db)
+ *((unsigned int *)db) = 0;
 
  return db;
 }

providers/hns/hns_roce_u_hw_v2.c

@@ -507,7 +507,7 @@ static void parse_cqe_for_srq(struct hns_roce_v2_cqe *cqe, struct ibv_wc *wc,
  handle_recv_cqe_inl_from_srq(cqe, srq);
 }
 
-static int parse_cqe_for_resp(struct hns_roce_v2_cqe *cqe, struct ibv_wc *wc,
+static void parse_cqe_for_resp(struct hns_roce_v2_cqe *cqe, struct ibv_wc *wc,
  struct hns_roce_qp *hr_qp)
 {
  struct hns_roce_wq *wq;
@@ -523,8 +523,6 @@ static int parse_cqe_for_resp(struct hns_roce_v2_cqe *cqe, struct ibv_wc *wc,
  handle_recv_cqe_inl_from_rq(cqe, hr_qp);
  else if (hr_reg_read(cqe, CQE_RQ_INLINE))
  handle_recv_rq_inl(cqe, hr_qp);
-
- return 0;
 }
 
 static void parse_cqe_for_req(struct hns_roce_v2_cqe *cqe, struct ibv_wc *wc,
@@ -2106,8 +2104,6 @@ static void wr_set_sge_list_rc(struct ibv_qp_ex *ibv_qp, size_t num_sge,
 
  wqe->msg_len = htole32(qp->sge_info.total_len);
  hr_reg_write(wqe, RCWQE_SGE_NUM, qp->sge_info.valid_num);
-
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static void wr_send_rc(struct ibv_qp_ex *ibv_qp)
@@ -2246,8 +2242,8 @@ static void set_inline_data_list_rc(struct hns_roce_qp *qp,
 {
  unsigned int msg_len = qp->sge_info.total_len;
  void *dseg;
+ size_t i;
  int ret;
- int i;
 
  hr_reg_enable(wqe, RCWQE_INLINE);
 
@@ -2299,15 +2295,14 @@ static void wr_set_inline_data_rc(struct ibv_qp_ex *ibv_qp, void *addr,
 
  qp->sge_info.total_len = length;
  set_inline_data_list_rc(qp, wqe, 1, &buff);
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static void wr_set_inline_data_list_rc(struct ibv_qp_ex *ibv_qp, size_t num_buf,
  const struct ibv_data_buf *buf_list)
 {
  struct hns_roce_qp *qp = to_hr_qp(&ibv_qp->qp_base);
  struct hns_roce_rc_sq_wqe *wqe = qp->cur_wqe;
- int i;
+ size_t i;
 
  if (!wqe)
  return;
@@ -2317,7 +2312,6 @@ static void wr_set_inline_data_list_rc(struct ibv_qp_ex *ibv_qp, size_t num_buf,
  qp->sge_info.total_len += buf_list[i].length;
 
  set_inline_data_list_rc(qp, wqe, num_buf, buf_list);
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static struct hns_roce_ud_sq_wqe *
@@ -2438,7 +2432,7 @@ static void wr_set_sge_list_ud(struct ibv_qp_ex *ibv_qp, size_t num_sge,
  }
 
  hr_reg_write(wqe, UDWQE_MSG_START_SGE_IDX, sge_idx & mask);
- for (int i = 0; i < num_sge; i++) {
+ for (size_t i = 0; i < num_sge; i++) {
  if (!sg_list[i].length)
  continue;
 
@@ -2454,7 +2448,6 @@ static void wr_set_sge_list_ud(struct ibv_qp_ex *ibv_qp, size_t num_sge,
  hr_reg_write(wqe, UDWQE_SGE_NUM, cnt);
 
  qp->sge_info.start_idx += cnt;
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static void set_inline_data_list_ud(struct hns_roce_qp *qp,
@@ -2465,8 +2458,8 @@ static void set_inline_data_list_ud(struct hns_roce_qp *qp,
  uint8_t data[HNS_ROCE_MAX_UD_INL_INN_SZ] = {};
  unsigned int msg_len = qp->sge_info.total_len;
  void *tmp;
+ size_t i;
  int ret;
- int i;
 
  if (!check_inl_data_len(qp, msg_len)) {
  qp->err = EINVAL;
@@ -2520,15 +2513,14 @@ static void wr_set_inline_data_ud(struct ibv_qp_ex *ibv_qp, void *addr,
 
  qp->sge_info.total_len = length;
  set_inline_data_list_ud(qp, wqe, 1, &buff);
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static void wr_set_inline_data_list_ud(struct ibv_qp_ex *ibv_qp, size_t num_buf,
  const struct ibv_data_buf *buf_list)
 {
  struct hns_roce_qp *qp = to_hr_qp(&ibv_qp->qp_base);
  struct hns_roce_ud_sq_wqe *wqe = qp->cur_wqe;
- int i;
+ size_t i;
 
  if (!wqe)
  return;
@@ -2538,7 +2530,6 @@ static void wr_set_inline_data_list_ud(struct ibv_qp_ex *ibv_qp, size_t num_buf,
  qp->sge_info.total_len += buf_list[i].length;
 
  set_inline_data_list_ud(qp, wqe, num_buf, buf_list);
- enable_wqe(qp, wqe, qp->sq.head);
 }
 
 static void wr_start(struct ibv_qp_ex *ibv_qp)

providers/hns/hns_roce_u_verbs.c

@@ -371,8 +371,6 @@ static struct ibv_cq_ex *create_cq(struct ibv_context *context,
  goto err_db;
  }
 
- *cq->db = 0;
-
  ret = exec_cq_create_cmd(context, cq, attr);
  if (ret)
  goto err_cmd;
@@ -385,8 +383,9 @@ static struct ibv_cq_ex *create_cq(struct ibv_context *context,
  hns_roce_free_db(hr_ctx, cq->db, HNS_ROCE_CQ_TYPE_DB);
 err_db:
  hns_roce_free_buf(&cq->buf);
-err_lock:
 err_buf:
+ pthread_spin_destroy(&cq->lock);
+err_lock:
  free(cq);
 err:
  if (ret < 0)
@@ -437,16 +436,20 @@ int hns_roce_u_modify_cq(struct ibv_cq *cq, struct ibv_modify_cq_attr *attr)
 
 int hns_roce_u_destroy_cq(struct ibv_cq *cq)
 {
+ struct hns_roce_cq *hr_cq = to_hr_cq(cq);
  int ret;
 
  ret = ibv_cmd_destroy_cq(cq);
  if (ret)
  return ret;
 
- hns_roce_free_db(to_hr_ctx(cq->context), to_hr_cq(cq)->db,
- HNS_ROCE_CQ_TYPE_DB);
- hns_roce_free_buf(&to_hr_cq(cq)->buf);
- free(to_hr_cq(cq));
+ hns_roce_free_db(to_hr_ctx(cq->context), hr_cq->db,
+ HNS_ROCE_CQ_TYPE_DB);
+ hns_roce_free_buf(&hr_cq->buf);
+
+ pthread_spin_destroy(&hr_cq->lock);
+
+ free(hr_cq);
 
  return ret;
 }
@@ -674,14 +677,12 @@ static struct ibv_srq *create_srq(struct ibv_context *context,
 
  set_srq_param(context, srq, init_attr);
  if (alloc_srq_buf(srq))
- goto err_free_srq;
+ goto err_destroy_lock;
 
  srq->rdb = hns_roce_alloc_db(hr_ctx, HNS_ROCE_SRQ_TYPE_DB);
  if (!srq->rdb)
  goto err_srq_buf;
 
- *srq->rdb = 0;
-
  ret = exec_srq_create_cmd(context, srq, init_attr);
  if (ret)
  goto err_srq_db;
@@ -705,6 +706,9 @@ static struct ibv_srq *create_srq(struct ibv_context *context,
 err_srq_buf:
  free_srq_buf(srq);
 
+err_destroy_lock:
+ pthread_spin_destroy(&srq->lock);
+
 err_free_srq:
  free(srq);
 
@@ -780,6 +784,8 @@ int hns_roce_u_destroy_srq(struct ibv_srq *ibv_srq)
 
  hns_roce_free_db(ctx, srq->rdb, HNS_ROCE_SRQ_TYPE_DB);
  free_srq_buf(srq);
+
+ pthread_spin_destroy(&srq->lock);
  free(srq);
 
  return 0;
@@ -1165,8 +1171,6 @@ static int qp_alloc_db(struct ibv_qp_init_attr_ex *attr, struct hns_roce_qp *qp,
  qp->sdb = hns_roce_alloc_db(ctx, HNS_ROCE_QP_TYPE_DB);
  if (!qp->sdb)
  return -ENOMEM;
-
- *qp->sdb = 0;
  }
 
  if (attr->cap.max_recv_sge) {
@@ -1178,8 +1182,6 @@ static int qp_alloc_db(struct ibv_qp_init_attr_ex *attr, struct hns_roce_qp *qp,
 
  return -ENOMEM;
  }
-
- *qp->rdb = 0;
  }
 
  return 0;
@@ -1295,6 +1297,8 @@ void hns_roce_free_qp_buf(struct hns_roce_qp *qp, struct hns_roce_context *ctx)
 {
  qp_free_db(qp, ctx);
  qp_free_wqe(qp);
+ pthread_spin_destroy(&qp->rq.lock);
+ pthread_spin_destroy(&qp->sq.lock);
 }
 
 static int hns_roce_alloc_qp_buf(struct ibv_qp_init_attr_ex *attr,
@@ -1303,17 +1307,30 @@ static int hns_roce_alloc_qp_buf(struct ibv_qp_init_attr_ex *attr,
 {
  int ret;
 
- if (pthread_spin_init(&qp->sq.lock, PTHREAD_PROCESS_PRIVATE) ||
- pthread_spin_init(&qp->rq.lock, PTHREAD_PROCESS_PRIVATE))
- return -ENOMEM;
+ ret = pthread_spin_init(&qp->sq.lock, PTHREAD_PROCESS_PRIVATE);
+ if (ret)
+ return ret;
+
+ ret = pthread_spin_init(&qp->rq.lock, PTHREAD_PROCESS_PRIVATE);
+ if (ret)
+ goto err_rq_lock;
 
  ret = qp_alloc_wqe(&attr->cap, qp, ctx);
  if (ret)
- return ret;
+ goto err_wqe;
 
  ret = qp_alloc_db(attr, qp, ctx);
  if (ret)
- qp_free_wqe(qp);
+ goto err_db;
+
+ return 0;
+
+err_db:
+ qp_free_wqe(qp);
+err_wqe:
+ pthread_spin_destroy(&qp->rq.lock);
+err_rq_lock:
+ pthread_spin_destroy(&qp->sq.lock);
 
  return ret;
 }