chenBright commented on code in PR #3034: URL: https://github.com/apache/brpc/pull/3034#discussion_r2218767893
########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -920,10 +923,10 @@ int RdmaEndpoint::SendImm(uint32_t imm) { wr.send_flags |= IBV_SEND_SIGNALED; ibv_send_wr* bad = NULL; - if (ibv_post_send(_resource->qp, &wr, &bad) < 0) { + if (int err = ibv_post_send(_resource->qp, &wr, &bad)) { // We use other way to guarantee the Send Queue is not full. // So we just consider this error as an unrecoverable error. - PLOG(WARNING) << "Fail to ibv_post_send"; + PLOG(WARNING) << "Fail to ibv_post_send: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -878,10 +878,13 @@ ssize_t RdmaEndpoint::CutFromIOBufList(butil::IOBuf** from, size_t ndata) { } ibv_send_wr* bad = NULL; - if (ibv_post_send(_resource->qp, &wr, &bad) < 0) { + if (int err = ibv_post_send(_resource->qp, &wr, &bad)) { // We use other way to guarantee the Send Queue is not full. // So we just consider this error as an unrecoverable error. - PLOG(WARNING) << "Fail to ibv_post_send"; + PLOG(WARNING) << "Fail to ibv_post_send: " << berror(err) Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1004,8 +1007,8 @@ int RdmaEndpoint::DoPostRecv(void* block, size_t block_size) { wr.sg_list = &sge; ibv_recv_wr* bad = NULL; - if (ibv_post_recv(_resource->qp, &wr, &bad) < 0) { - PLOG(WARNING) << "Fail to ibv_post_recv"; + if (int err = ibv_post_recv(_resource->qp, &wr, &bad)) { + PLOG(WARNING) << "Fail to ibv_post_recv: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1143,8 +1146,8 @@ int RdmaEndpoint::AllocateResources() { return -1; } - if (ibv_req_notify_cq(_resource->cq, 1) < 0) { - PLOG(WARNING) << "Fail to arm CQ comp channel"; + if (int err = ibv_req_notify_cq(_resource->cq, 1)) { + PLOG(WARNING) << "Fail to arm CQ comp channel: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1186,12 +1189,12 @@ int RdmaEndpoint::BringUpQp(uint16_t lid, ibv_gid gid, uint32_t qp_num) { attr.pkey_index = 0; // TODO: support more pkey use in future attr.port_num = GetRdmaPortNum(); attr.qp_access_flags = IBV_ACCESS_REMOTE_WRITE; - if (IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( + if (int err = IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( IBV_QP_STATE | IBV_QP_PKEY_INDEX | IBV_QP_PORT | - IBV_QP_ACCESS_FLAGS)) < 0) { - PLOG(WARNING) << "Fail to modify QP from RESET to INIT"; + IBV_QP_ACCESS_FLAGS))) { + PLOG(WARNING) << "Fail to modify QP from RESET to INIT: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1217,15 +1220,15 @@ int RdmaEndpoint::BringUpQp(uint16_t lid, ibv_gid gid, uint32_t qp_num) { attr.rq_psn = 0; attr.max_dest_rd_atomic = 0; attr.min_rnr_timer = 0; // We do not allow rnr error - if (IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( + if (int err = IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( IBV_QP_STATE | IBV_QP_PATH_MTU | IBV_QP_MIN_RNR_TIMER | IBV_QP_AV | IBV_QP_MAX_DEST_RD_ATOMIC | IBV_QP_DEST_QPN | - IBV_QP_RQ_PSN)) < 0) { - PLOG(WARNING) << "Fail to modify QP from INIT to RTR"; + IBV_QP_RQ_PSN))) { + PLOG(WARNING) << "Fail to modify QP from INIT to RTR: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1272,15 +1275,15 @@ void RdmaEndpoint::DeallocateResources() { } if (!move_to_rdma_resource_list) { if (_resource->qp) { - if (IbvDestroyQp(_resource->qp) < 0) { - PLOG(WARNING) << "Fail to destroy QP"; + if (int err = IbvDestroyQp(_resource->qp)) { + PLOG(WARNING) << "Fail to destroy QP: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1272,15 +1275,15 @@ void RdmaEndpoint::DeallocateResources() { } if (!move_to_rdma_resource_list) { if (_resource->qp) { - if (IbvDestroyQp(_resource->qp) < 0) { - PLOG(WARNING) << "Fail to destroy QP"; + if (int err = IbvDestroyQp(_resource->qp)) { + PLOG(WARNING) << "Fail to destroy QP: " << berror(err); } _resource->qp = NULL; } if (_resource->cq) { IbvAckCqEvents(_resource->cq, _cq_events); - if (IbvDestroyCq(_resource->cq) < 0) { - PLOG(WARNING) << "Fail to destroy CQ"; + if (int err = IbvDestroyCq(_resource->cq)) { + PLOG(WARNING) << "Fail to destroy CQ: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1235,14 +1238,14 @@ int RdmaEndpoint::BringUpQp(uint16_t lid, ibv_gid gid, uint32_t qp_num) { attr.rnr_retry = 0; // We do not allow rnr error attr.sq_psn = 0; attr.max_rd_atomic = 0; - if (IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( + if (int err = IbvModifyQp(_resource->qp, &attr, (ibv_qp_attr_mask)( IBV_QP_STATE | IBV_QP_RNR_RETRY | IBV_QP_RETRY_CNT | IBV_QP_TIMEOUT | IBV_QP_SQ_PSN | - IBV_QP_MAX_QP_RD_ATOMIC)) < 0) { - PLOG(WARNING) << "Fail to modify QP from RTR to RTS"; + IBV_QP_MAX_QP_RD_ATOMIC))) { + PLOG(WARNING) << "Fail to modify QP from RTR to RTS: " << berror(err); Review Comment: PLOG -> LOG ########## src/brpc/rdma/rdma_endpoint.cpp: ########## @@ -1289,8 +1292,8 @@ void RdmaEndpoint::DeallocateResources() { // so that we should remove it from epoll fd first _socket->_io_event.RemoveConsumer(fd); fd = -1; - if (IbvDestroyCompChannel(_resource->comp_channel) < 0) { - PLOG(WARNING) << "Fail to destroy CQ channel"; + if (int err = IbvDestroyCompChannel(_resource->comp_channel)) { + PLOG(WARNING) << "Fail to destroy CQ channel: " << berror(err); Review Comment: PLOG -> LOG -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@brpc.apache.org For additional commands, e-mail: dev-h...@brpc.apache.org