Message ID | 33fe3d5bd6223d0ca1b5002efb7efc7bb90f3495.1683841772.git.quic_nguyenb@quicinc.com (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | ufs: core: mcq: Add ufshcd_abort() and error handler support in MCQ mode | expand |
Hi Bao, kernel test robot noticed the following build warnings: [auto build test WARNING on jejb-scsi/for-next] [also build test WARNING on mkp-scsi/for-next linus/master v6.4-rc1 next-20230511] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bao-D-Nguyen/ufs-core-Combine-32-bit-command_desc_base_addr_lo-hi/20230512-060009 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next patch link: https://lore.kernel.org/r/33fe3d5bd6223d0ca1b5002efb7efc7bb90f3495.1683841772.git.quic_nguyenb%40quicinc.com patch subject: [PATCH v4 4/7] ufs: mcq: Add support for clean up mcq resources config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20230512/202305121123.KOfhO8tv-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/e05a8eb3fd257b04965c2333d4bf0161177ef504 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Bao-D-Nguyen/ufs-core-Combine-32-bit-command_desc_base_addr_lo-hi/20230512-060009 git checkout e05a8eb3fd257b04965c2333d4bf0161177ef504 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/ufs/core/ If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202305121123.KOfhO8tv-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/ufs/core/ufshcd.c:7526:32: warning: variable 'reg' is uninitialized when used here [-Wuninitialized] if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) { ^~~ drivers/ufs/core/ufshcd.c:7489:9: note: initialize the variable 'reg' to silence this warning u32 reg; ^ = 0 1 warning generated. vim +/reg +7526 drivers/ufs/core/ufshcd.c 7473 7474 /** 7475 * ufshcd_abort - scsi host template eh_abort_handler callback 7476 * @cmd: SCSI command pointer 7477 * 7478 * Returns SUCCESS/FAILED 7479 */ 7480 static int ufshcd_abort(struct scsi_cmnd *cmd) 7481 { 7482 struct Scsi_Host *host = cmd->device->host; 7483 struct ufs_hba *hba = shost_priv(host); 7484 int tag = scsi_cmd_to_rq(cmd)->tag; 7485 struct ufshcd_lrb *lrbp = &hba->lrb[tag]; 7486 unsigned long flags; 7487 int err = FAILED; 7488 bool outstanding; 7489 u32 reg; 7490 7491 WARN_ONCE(tag < 0, "Invalid tag %d\n", tag); 7492 7493 ufshcd_hold(hba, false); 7494 7495 if (!is_mcq_enabled(hba) && !test_bit(tag, &hba->outstanding_reqs)) { 7496 reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); 7497 /* If command is already aborted/completed, return FAILED. */ 7498 dev_err(hba->dev, 7499 "%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n", 7500 __func__, tag, hba->outstanding_reqs, reg); 7501 goto release; 7502 } 7503 7504 /* Print Transfer Request of aborted task */ 7505 dev_info(hba->dev, "%s: Device abort task at tag %d\n", __func__, tag); 7506 7507 /* 7508 * Print detailed info about aborted request. 7509 * As more than one request might get aborted at the same time, 7510 * print full information only for the first aborted request in order 7511 * to reduce repeated printouts. For other aborted requests only print 7512 * basic details. 7513 */ 7514 scsi_print_command(cmd); 7515 if (!hba->req_abort_count) { 7516 ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, tag); 7517 ufshcd_print_evt_hist(hba); 7518 ufshcd_print_host_state(hba); 7519 ufshcd_print_pwr_info(hba); 7520 ufshcd_print_tr(hba, tag, true); 7521 } else { 7522 ufshcd_print_tr(hba, tag, false); 7523 } 7524 hba->req_abort_count++; 7525 > 7526 if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) { 7527 /* only execute this code in single doorbell mode */ 7528 dev_err(hba->dev, 7529 "%s: cmd was completed, but without a notifying intr, tag = %d", 7530 __func__, tag); 7531 __ufshcd_transfer_req_compl(hba, 1UL << tag); 7532 goto release; 7533 } 7534 7535 /* 7536 * Task abort to the device W-LUN is illegal. When this command 7537 * will fail, due to spec violation, scsi err handling next step 7538 * will be to send LU reset which, again, is a spec violation. 7539 * To avoid these unnecessary/illegal steps, first we clean up 7540 * the lrb taken by this cmd and re-set it in outstanding_reqs, 7541 * then queue the eh_work and bail. 7542 */ 7543 if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) { 7544 ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun); 7545 7546 spin_lock_irqsave(host->host_lock, flags); 7547 hba->force_reset = true; 7548 ufshcd_schedule_eh_work(hba); 7549 spin_unlock_irqrestore(host->host_lock, flags); 7550 goto release; 7551 } 7552 7553 /* Skip task abort in case previous aborts failed and report failure */ 7554 if (lrbp->req_abort_skip) { 7555 dev_err(hba->dev, "%s: skipping abort\n", __func__); 7556 ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs); 7557 goto release; 7558 } 7559 7560 err = ufshcd_try_to_abort_task(hba, tag); 7561 if (err) { 7562 dev_err(hba->dev, "%s: failed with err %d\n", __func__, err); 7563 ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs); 7564 err = FAILED; 7565 goto release; 7566 } 7567 7568 /* 7569 * Clear the corresponding bit from outstanding_reqs since the command 7570 * has been aborted successfully. 7571 */ 7572 spin_lock_irqsave(&hba->outstanding_lock, flags); 7573 outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs); 7574 spin_unlock_irqrestore(&hba->outstanding_lock, flags); 7575 7576 if (outstanding) 7577 ufshcd_release_scsi_cmd(hba, lrbp); 7578 7579 err = SUCCESS; 7580 7581 release: 7582 /* Matches the ufshcd_hold() call at the start of this function. */ 7583 ufshcd_release(hba); 7584 return err; 7585 } 7586
Hi Bao, kernel test robot noticed the following build warnings: [auto build test WARNING on jejb-scsi/for-next] [also build test WARNING on mkp-scsi/for-next linus/master v6.4-rc1 next-20230511] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/Bao-D-Nguyen/ufs-core-Combine-32-bit-command_desc_base_addr_lo-hi/20230512-060009 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next patch link: https://lore.kernel.org/r/33fe3d5bd6223d0ca1b5002efb7efc7bb90f3495.1683841772.git.quic_nguyenb%40quicinc.com patch subject: [PATCH v4 4/7] ufs: mcq: Add support for clean up mcq resources config: x86_64-randconfig-a014 (https://download.01.org/0day-ci/archive/20230512/202305121245.uSA6swAd-lkp@intel.com/config) compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/e05a8eb3fd257b04965c2333d4bf0161177ef504 git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Bao-D-Nguyen/ufs-core-Combine-32-bit-command_desc_base_addr_lo-hi/20230512-060009 git checkout e05a8eb3fd257b04965c2333d4bf0161177ef504 # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot <lkp@intel.com> | Link: https://lore.kernel.org/oe-kbuild-all/202305121245.uSA6swAd-lkp@intel.com/ All warnings (new ones prefixed by >>): >> drivers/ufs/core/ufshcd.c:7526:32: warning: variable 'reg' is uninitialized when used here [-Wuninitialized] if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) { ^~~ drivers/ufs/core/ufshcd.c:7489:9: note: initialize the variable 'reg' to silence this warning u32 reg; ^ = 0 1 warning generated. vim +/reg +7526 drivers/ufs/core/ufshcd.c 7473 7474 /** 7475 * ufshcd_abort - scsi host template eh_abort_handler callback 7476 * @cmd: SCSI command pointer 7477 * 7478 * Returns SUCCESS/FAILED 7479 */ 7480 static int ufshcd_abort(struct scsi_cmnd *cmd) 7481 { 7482 struct Scsi_Host *host = cmd->device->host; 7483 struct ufs_hba *hba = shost_priv(host); 7484 int tag = scsi_cmd_to_rq(cmd)->tag; 7485 struct ufshcd_lrb *lrbp = &hba->lrb[tag]; 7486 unsigned long flags; 7487 int err = FAILED; 7488 bool outstanding; 7489 u32 reg; 7490 7491 WARN_ONCE(tag < 0, "Invalid tag %d\n", tag); 7492 7493 ufshcd_hold(hba, false); 7494 7495 if (!is_mcq_enabled(hba) && !test_bit(tag, &hba->outstanding_reqs)) { 7496 reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); 7497 /* If command is already aborted/completed, return FAILED. */ 7498 dev_err(hba->dev, 7499 "%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n", 7500 __func__, tag, hba->outstanding_reqs, reg); 7501 goto release; 7502 } 7503 7504 /* Print Transfer Request of aborted task */ 7505 dev_info(hba->dev, "%s: Device abort task at tag %d\n", __func__, tag); 7506 7507 /* 7508 * Print detailed info about aborted request. 7509 * As more than one request might get aborted at the same time, 7510 * print full information only for the first aborted request in order 7511 * to reduce repeated printouts. For other aborted requests only print 7512 * basic details. 7513 */ 7514 scsi_print_command(cmd); 7515 if (!hba->req_abort_count) { 7516 ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, tag); 7517 ufshcd_print_evt_hist(hba); 7518 ufshcd_print_host_state(hba); 7519 ufshcd_print_pwr_info(hba); 7520 ufshcd_print_tr(hba, tag, true); 7521 } else { 7522 ufshcd_print_tr(hba, tag, false); 7523 } 7524 hba->req_abort_count++; 7525 > 7526 if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) { 7527 /* only execute this code in single doorbell mode */ 7528 dev_err(hba->dev, 7529 "%s: cmd was completed, but without a notifying intr, tag = %d", 7530 __func__, tag); 7531 __ufshcd_transfer_req_compl(hba, 1UL << tag); 7532 goto release; 7533 } 7534 7535 /* 7536 * Task abort to the device W-LUN is illegal. When this command 7537 * will fail, due to spec violation, scsi err handling next step 7538 * will be to send LU reset which, again, is a spec violation. 7539 * To avoid these unnecessary/illegal steps, first we clean up 7540 * the lrb taken by this cmd and re-set it in outstanding_reqs, 7541 * then queue the eh_work and bail. 7542 */ 7543 if (lrbp->lun == UFS_UPIU_UFS_DEVICE_WLUN) { 7544 ufshcd_update_evt_hist(hba, UFS_EVT_ABORT, lrbp->lun); 7545 7546 spin_lock_irqsave(host->host_lock, flags); 7547 hba->force_reset = true; 7548 ufshcd_schedule_eh_work(hba); 7549 spin_unlock_irqrestore(host->host_lock, flags); 7550 goto release; 7551 } 7552 7553 /* Skip task abort in case previous aborts failed and report failure */ 7554 if (lrbp->req_abort_skip) { 7555 dev_err(hba->dev, "%s: skipping abort\n", __func__); 7556 ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs); 7557 goto release; 7558 } 7559 7560 err = ufshcd_try_to_abort_task(hba, tag); 7561 if (err) { 7562 dev_err(hba->dev, "%s: failed with err %d\n", __func__, err); 7563 ufshcd_set_req_abort_skip(hba, hba->outstanding_reqs); 7564 err = FAILED; 7565 goto release; 7566 } 7567 7568 /* 7569 * Clear the corresponding bit from outstanding_reqs since the command 7570 * has been aborted successfully. 7571 */ 7572 spin_lock_irqsave(&hba->outstanding_lock, flags); 7573 outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs); 7574 spin_unlock_irqrestore(&hba->outstanding_lock, flags); 7575 7576 if (outstanding) 7577 ufshcd_release_scsi_cmd(hba, lrbp); 7578 7579 err = SUCCESS; 7580 7581 release: 7582 /* Matches the ufshcd_hold() call at the start of this function. */ 7583 ufshcd_release(hba); 7584 return err; 7585 } 7586
diff --git a/drivers/ufs/core/ufshcd-priv.h b/drivers/ufs/core/ufshcd-priv.h index 40727e8..3f518e9 100644 --- a/drivers/ufs/core/ufshcd-priv.h +++ b/drivers/ufs/core/ufshcd-priv.h @@ -78,6 +78,7 @@ struct ufs_hw_queue *ufshcd_mcq_req_to_hwq(struct ufs_hba *hba, unsigned long ufshcd_mcq_poll_cqe_lock(struct ufs_hba *hba, struct ufs_hw_queue *hwq); +bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd); int ufshcd_mcq_sq_cleanup(struct ufs_hba *hba, int task_tag); #define UFSHCD_MCQ_IO_QUEUE_OFFSET 1 diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 140ab15..585f530 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -3005,6 +3005,26 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba, } /* + * Check with the block layer if the command is inflight + * @cmd: command to check. + * + * Returns true if command is inflight; false if not. + */ +bool ufshcd_cmd_inflight(struct scsi_cmnd *cmd) +{ + struct request *rq; + + if (!cmd) + return false; + + rq = scsi_cmd_to_rq(cmd); + if (!blk_mq_request_started(rq)) + return false; + + return true; +} + +/* * Clear the pending command in the controller and wait until * the controller confirms that the command has been cleared. * @hba: per adapter instance @@ -3012,8 +3032,23 @@ static int ufshcd_compose_dev_cmd(struct ufs_hba *hba, */ static int ufshcd_clear_cmd(struct ufs_hba *hba, u32 task_tag) { - unsigned long flags; u32 mask = 1U << task_tag; + unsigned long flags; + int err; + + if (is_mcq_enabled(hba)) { + /* + * MCQ mode. Clean up the MCQ resources similar to + * what the ufshcd_utrl_clear() does for SDB mode. + */ + err = ufshcd_mcq_sq_cleanup(hba, task_tag); + if (err) { + dev_err(hba->dev, "%s: failed tag=%d. err=%d\n", + __func__, task_tag, err); + return err; + } + return 0; + } /* clear outstanding transaction before retry */ spin_lock_irqsave(hba->host->host_lock, flags); @@ -7384,6 +7419,20 @@ static int ufshcd_try_to_abort_task(struct ufs_hba *hba, int tag) */ dev_err(hba->dev, "%s: cmd at tag %d not pending in the device.\n", __func__, tag); + if (is_mcq_enabled(hba)) { + /* MCQ mode */ + if (ufshcd_cmd_inflight(lrbp->cmd)) { + /* sleep for max. 200us same delay as in SDB mode */ + usleep_range(100, 200); + continue; + } + /* command completed already */ + dev_err(hba->dev, "%s: cmd at tag=%d is cleared.\n", + __func__, tag); + goto out; + } + + /* Single Doorbell Mode */ reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); if (reg & (1 << tag)) { /* sleep for max. 200us to stabilize */ @@ -7449,9 +7498,10 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) WARN_ONCE(tag < 0, "Invalid tag %d\n", tag); ufshcd_hold(hba, false); - reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); - /* If command is already aborted/completed, return FAILED. */ - if (!(test_bit(tag, &hba->outstanding_reqs))) { + + if (!is_mcq_enabled(hba) && !test_bit(tag, &hba->outstanding_reqs)) { + reg = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL); + /* If command is already aborted/completed, return FAILED. */ dev_err(hba->dev, "%s: cmd at tag %d already completed, outstanding=0x%lx, doorbell=0x%x\n", __func__, tag, hba->outstanding_reqs, reg); @@ -7480,7 +7530,8 @@ static int ufshcd_abort(struct scsi_cmnd *cmd) } hba->req_abort_count++; - if (!(reg & (1 << tag))) { + if (!is_mcq_enabled(hba) && !(reg & (1 << tag))) { + /* only execute this code in single doorbell mode */ dev_err(hba->dev, "%s: cmd was completed, but without a notifying intr, tag = %d", __func__, tag);
Update ufshcd_clear_cmd() to clean up the mcq resources similar to the function ufshcd_utrl_clear() does for sdb mode. Update ufshcd_try_to_abort_task() to support mcq mode so that this function can be invoked in either mcq or sdb mode. Signed-off-by: Bao D. Nguyen <quic_nguyenb@quicinc.com> --- drivers/ufs/core/ufshcd-priv.h | 1 + drivers/ufs/core/ufshcd.c | 61 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 5 deletions(-)