Hello Kashyap Desai,
The patch 691eb7c6110f: "RDMA/bnxt_re: handle command completions
after driver detect a timedout" from Jun 9, 2023, leads to the
following Smatch static checker warning:
drivers/infiniband/hw/bnxt_re/qplib_rcfw.c:513 __bnxt_qplib_rcfw_send_message()
warn: duplicate check 'rc' (previous on line 506)
drivers/infiniband/hw/bnxt_re/qplib_rcfw.c
477 static int __bnxt_qplib_rcfw_send_message(struct bnxt_qplib_rcfw *rcfw,
478 struct bnxt_qplib_cmdqmsg *msg)
479 {
480 struct creq_qp_event *evnt = (struct creq_qp_event *)msg->resp;
481 struct bnxt_qplib_crsqe *crsqe;
482 unsigned long flags;
483 u16 cookie;
484 int rc = 0;
485 u8 opcode;
486
487 opcode = __get_cmdq_base_opcode(msg->req, msg->req_sz);
488
489 rc = __send_message_basic_sanity(rcfw, msg, opcode);
490 if (rc)
491 return rc == -ENXIO ? bnxt_qplib_map_rc(opcode) : rc;
The comments in bnxt_qplib_map_rc() talk about "firmware halt is detected"
and "this function will be called when FW already reports a timeout.
This would be possible only when FW crashes and resets." The
__send_message_basic_sanity() function returns -ENXIO in the case of
ERR_DEVICE_DETACHED and it returns -ETIMEDOUT in the case of
FIRMWARE_STALL_DETECTED.
Based on the comments, I would have expected this test to be:
if (rc)
return rc == -ETIMEDOUT ? bnxt_qplib_map_rc(opcode) : rc;
Presumably the code is correct, but the comments are confusing.
492
493 rc = __send_message(rcfw, msg);
494 if (rc)
495 return rc;
496
497 cookie = le16_to_cpu(__get_cmdq_base_cookie(msg->req, msg->req_sz))
498 & RCFW_MAX_COOKIE_VALUE;
499
500 if (msg->block)
501 rc = __block_for_resp(rcfw, cookie);
502 else if (atomic_read(&rcfw->rcfw_intr_enabled))
503 rc = __wait_for_resp(rcfw, cookie);
504 else
505 rc = __poll_for_resp(rcfw, cookie);
506 if (rc) {
^^^^^^^^^
rc is checked here.
507 /* timed out */
508 dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x timedout (%d)msec\n",
509 cookie, opcode, RCFW_CMD_WAIT_TIME_MS);
510 return rc;
511 }
512
--> 513 if (rc) {
^^^^^^^^^
The patch adds some dead code.
514 spin_lock_irqsave(&rcfw->cmdq.hwq.lock, flags);
515 crsqe = &rcfw->crsqe_tbl[cookie];
516 crsqe->is_waiter_alive = false;
517 if (rc == -ENODEV)
518 set_bit(FIRMWARE_STALL_DETECTED, &rcfw->cmdq.flags);
519 spin_unlock_irqrestore(&rcfw->cmdq.hwq.lock, flags);
520 return -ETIMEDOUT;
521 }
522
523 if (evnt->status) {
524 /* failed with status */
525 dev_err(&rcfw->pdev->dev, "cmdq[%#x]=%#x status %#x\n",
526 cookie, opcode, evnt->status);
527 rc = -EFAULT;
528 }
529
530 return rc;
531 }
regards,
dan carpenter
