* [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information [not found] <CGME20200620070044epcas2p269e3c266c86c65dd0e894d8188036a30@epcas2p2.samsung.com> @ 2020-06-20 6:53 ` Kiwoong Kim [not found] ` <CGME20200620070047epcas2p37229d52d479df9f64ee4fc14f469acf9@epcas2p3.samsung.com> ` (3 more replies) 0 siblings, 4 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-20 6:53 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche Cc: Kiwoong Kim Some SoC specific might need command history for various reasons, such as stacking command contexts in system memory to check for debugging in the future or scaling some DVFS knobs to boost IO throughput. What you would do with the information could be variant per SoC vendor. Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> --- drivers/scsi/ufs/ufshcd.c | 4 ++++ drivers/scsi/ufs/ufshcd.h | 8 ++++++++ 2 files changed, 12 insertions(+) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 52abe82..0eae3ce 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) /* issue command to the controller */ spin_lock_irqsave(hba->host->host_lock, flags); ufshcd_vops_setup_xfer_req(hba, tag, true); + if (cmd) + ufshcd_vops_cmd_log(hba, cmd, 1); ufshcd_send_command(hba, tag); out_unlock: spin_unlock_irqrestore(hba->host->host_lock, flags); @@ -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba, /* Mark completed command as NULL in LRB */ lrbp->cmd = NULL; lrbp->compl_time_stamp = ktime_get(); + ufshcd_vops_cmd_log(hba, cmd, 2); + /* Do not touch lrbp after scsi done */ cmd->scsi_done(cmd); __ufshcd_release(hba); diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h index c774012..80c4f0d 100644 --- a/drivers/scsi/ufs/ufshcd.h +++ b/drivers/scsi/ufs/ufshcd.h @@ -307,6 +307,7 @@ struct ufs_hba_variant_ops { void (*config_scaling_param)(struct ufs_hba *hba, struct devfreq_dev_profile *profile, void *data); + void (*cmd_log)(struct ufs_hba *hba, struct scsi_cmnd *cmd, int enter); }; /* clock gating state */ @@ -1137,6 +1138,13 @@ static inline void ufshcd_vops_config_scaling_param(struct ufs_hba *hba, hba->vops->config_scaling_param(hba, profile, data); } +static inline void ufshcd_vops_cmd_log(struct ufs_hba *hba, + struct scsi_cmnd *cmd, int enter) +{ + if (hba->vops && hba->vops->cmd_log) + hba->vops->cmd_log(hba, cmd, enter); +} + extern struct ufs_pm_lvl_states ufs_pm_lvl_states[]; /* -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
[parent not found: <CGME20200620070047epcas2p37229d52d479df9f64ee4fc14f469acf9@epcas2p3.samsung.com>]
* [RFC PATCH v1 2/2] exynos-ufs: support command history [not found] ` <CGME20200620070047epcas2p37229d52d479df9f64ee4fc14f469acf9@epcas2p3.samsung.com> @ 2020-06-20 6:53 ` Kiwoong Kim 2020-06-20 15:26 ` kernel test robot ` (2 more replies) 0 siblings, 3 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-20 6:53 UTC (permalink / raw) To: linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, bvanassche Cc: Kiwoong Kim This patch is to record contexts of incoming commands in a circular queue. ufshcd.c has already some function tracer calls to get command history but ftrace would be gone when system dies before you get the information, such as panic cases. Setting EN_PRINT_UFS_LOG to one enables to print information in the circular queue whenever ufshcd.c invokes a dbg_register_dump callback. Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> --- drivers/scsi/ufs/Makefile | 2 +- drivers/scsi/ufs/ufs-exynos-dbg.c | 208 ++++++++++++++++++++++++++++++++++++++ drivers/scsi/ufs/ufs-exynos-if.h | 19 ++++ drivers/scsi/ufs/ufs-exynos.c | 43 ++++++++ drivers/scsi/ufs/ufs-exynos.h | 13 +++ 5 files changed, 284 insertions(+), 1 deletion(-) create mode 100644 drivers/scsi/ufs/ufs-exynos-dbg.c create mode 100644 drivers/scsi/ufs/ufs-exynos-if.h diff --git a/drivers/scsi/ufs/Makefile b/drivers/scsi/ufs/Makefile index f0c5b95..d9e4da7 100644 --- a/drivers/scsi/ufs/Makefile +++ b/drivers/scsi/ufs/Makefile @@ -4,7 +4,7 @@ obj-$(CONFIG_SCSI_UFS_DWC_TC_PCI) += tc-dwc-g210-pci.o ufshcd-dwc.o tc-dwc-g210. obj-$(CONFIG_SCSI_UFS_DWC_TC_PLATFORM) += tc-dwc-g210-pltfrm.o ufshcd-dwc.o tc-dwc-g210.o obj-$(CONFIG_SCSI_UFS_CDNS_PLATFORM) += cdns-pltfrm.o obj-$(CONFIG_SCSI_UFS_QCOM) += ufs-qcom.o -obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o +obj-$(CONFIG_SCSI_UFS_EXYNOS) += ufs-exynos.o ufs-exynos-dbg.o obj-$(CONFIG_SCSI_UFSHCD) += ufshcd-core.o ufshcd-core-y += ufshcd.o ufs-sysfs.o ufshcd-core-$(CONFIG_SCSI_UFS_BSG) += ufs_bsg.o diff --git a/drivers/scsi/ufs/ufs-exynos-dbg.c b/drivers/scsi/ufs/ufs-exynos-dbg.c new file mode 100644 index 0000000..2265af7 --- /dev/null +++ b/drivers/scsi/ufs/ufs-exynos-dbg.c @@ -0,0 +1,208 @@ +// SPDX-License-Identifier: GPL-2.0-only +/* + * UFS Exynos debugging functions + * + * Copyright (C) 2020 Samsung Electronics Co., Ltd. + * Author: Kiwoong Kim <kwmad.kim@samsung.com> + * + */ +#include <linux/platform_device.h> +#include <linux/module.h> +#include "ufshcd.h" +#include "ufs-exynos.h" +#include "ufs-exynos-if.h" + +#define EN_PRINT_UFS_LOG 1 + +/* Structure for ufs cmd logging */ +#define MAX_CMD_LOGS 32 + +struct cmd_data { + unsigned int tag; + unsigned int sct; + unsigned long lba; + u64 start_time; + u64 end_time; + u64 outstanding_reqs; + int retries; + unsigned char op; +}; + +struct ufs_cmd_info { + u32 total; + u32 last; + struct cmd_data data[MAX_CMD_LOGS]; + struct cmd_data *pdata[32]; /* Currently, 32 slots */ +}; + +#define DBG_NUM_OF_HOSTS 1 +struct ufs_s_dbg_mgr { + struct ufs_exynos_handle *handle; + int active; + u64 first_time; + u64 time; + + /* cmd log */ + struct ufs_cmd_info cmd_info; + struct cmd_data cmd_log; /* temp buffer to put */ + spinlock_t cmd_lock; +}; +static struct ufs_s_dbg_mgr ufs_s_dbg[DBG_NUM_OF_HOSTS]; +static int ufs_s_dbg_mgr_idx; + +static void ufs_s_print_cmd_log(struct ufs_s_dbg_mgr *mgr, struct device *dev) +{ + struct ufs_cmd_info *cmd_info = &mgr->cmd_info; + struct cmd_data *data = cmd_info->data; + u32 i; + u32 last; + u32 max = MAX_CMD_LOGS; + unsigned long flags; + u32 total; + + spin_lock_irqsave(&mgr->cmd_lock, flags); + total = cmd_info->total; + if (cmd_info->total < max) + max = cmd_info->total; + last = (cmd_info->last + MAX_CMD_LOGS - 1) % MAX_CMD_LOGS; + spin_unlock_irqrestore(&mgr->cmd_lock, flags); + + dev_err(dev, ":---------------------------------------------------\n"); + dev_err(dev, ":\t\tSCSI CMD(%u)\n", total - 1); + dev_err(dev, ":---------------------------------------------------\n"); + dev_err(dev, ":OP, TAG, LBA, SCT, RETRIES, STIME, ETIME, REQS\n\n"); + + for (i = 0 ; i < max ; i++, data++) { + dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s", + data->op, + data->tag, + data->lba, + data->sct, + data->retries, + data->start_time, + data->end_time, + data->outstanding_reqs, + ((last == i) ? "<--" : " ")); + if (last == i) + dev_err(dev, "\n"); + } +} + +static void ufs_s_put_cmd_log(struct ufs_s_dbg_mgr *mgr, + struct cmd_data *cmd_data) +{ + struct ufs_cmd_info *cmd_info = &mgr->cmd_info; + unsigned long flags; + struct cmd_data *pdata; + + spin_lock_irqsave(&mgr->cmd_lock, flags); + pdata = &cmd_info->data[cmd_info->last]; + ++cmd_info->total; + ++cmd_info->last; + cmd_info->last = cmd_info->last % MAX_CMD_LOGS; + spin_unlock_irqrestore(&mgr->cmd_lock, flags); + + pdata->op = cmd_data->op; + pdata->tag = cmd_data->tag; + pdata->lba = cmd_data->lba; + pdata->sct = cmd_data->sct; + pdata->retries = cmd_data->retries; + pdata->start_time = cmd_data->start_time; + pdata->end_time = 0; + pdata->outstanding_reqs = cmd_data->outstanding_reqs; + cmd_info->pdata[cmd_data->tag] = pdata; +} + +/* + * EXTERNAL FUNCTIONS + * + * There are two classes that are to initialize data structures for debug + * and to define actual behavior. + */ +void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct device *dev) +{ + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; + + if (mgr->active == 0) + goto out; + + mgr->time = cpu_clock(raw_smp_processor_id()); + +#if EN_PRINT_UFS_LOG + ufs_s_print_cmd_log(mgr, dev); +#endif + + if (mgr->first_time == 0ULL) + mgr->first_time = mgr->time; +out: + return; +} + +void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle, + struct ufs_hba *hba, struct scsi_cmnd *cmd) +{ + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; + int cpu = raw_smp_processor_id(); + struct cmd_data *cmd_log = &mgr->cmd_log; /* temp buffer to put */ + unsigned long lba = (cmd->cmnd[2] << 24) | + (cmd->cmnd[3] << 16) | + (cmd->cmnd[4] << 8) | + (cmd->cmnd[5] << 0); + unsigned int sct = (cmd->cmnd[7] << 8) | + (cmd->cmnd[8] << 0); + + if (mgr->active == 0) + return; + + cmd_log->start_time = cpu_clock(cpu); + cmd_log->op = cmd->cmnd[0]; + cmd_log->tag = cmd->request->tag; + + /* This function runtime is protected by spinlock from outside */ + cmd_log->outstanding_reqs = hba->outstanding_reqs; + + /* unmap */ + if (cmd->cmnd[0] != UNMAP) + cmd_log->lba = lba; + + cmd_log->sct = sct; + cmd_log->retries = cmd->allowed; + + ufs_s_put_cmd_log(mgr, cmd_log); +} + +void exynos_ufs_cmd_log_end(struct ufs_exynos_handle *handle, + struct ufs_hba *hba, struct scsi_cmnd *cmd) +{ + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; + struct ufs_cmd_info *cmd_info = &mgr->cmd_info; + int cpu = raw_smp_processor_id(); + int tag = cmd->request->tag; + + if (mgr->active == 0) + return; + + cmd_info->pdata[tag]->end_time = cpu_clock(cpu); +} + +int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle) +{ + struct ufs_s_dbg_mgr *mgr; + + if (ufs_s_dbg_mgr_idx >= DBG_NUM_OF_HOSTS) + return -1; + + mgr = &ufs_s_dbg[ufs_s_dbg_mgr_idx++]; + handle->private = (void *)mgr; + mgr->handle = handle; + mgr->active = 1; + + /* cmd log */ + spin_lock_init(&mgr->cmd_lock); + + return 0; +} +MODULE_AUTHOR("Kiwoong Kim <kwmad.kim@samsung.com>"); +MODULE_DESCRIPTION("Exynos UFS debug information"); +MODULE_LICENSE("GPL"); +MODULE_VERSION("0.1"); diff --git a/drivers/scsi/ufs/ufs-exynos-if.h b/drivers/scsi/ufs/ufs-exynos-if.h new file mode 100644 index 0000000..33f7187 --- /dev/null +++ b/drivers/scsi/ufs/ufs-exynos-if.h @@ -0,0 +1,19 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * UFS Exynos debugging functions + * + * Copyright (C) 2020 Samsung Electronics Co., Ltd. + * Author: Kiwoong Kim <kwmad.kim@samsung.com> + * + */ +#ifndef _UFS_EXYNOS_IF_H_ +#define _UFS_EXYNOS_IF_H_ + +#define UFS_VS_MMIO_VER 1 + +/* more members would be added in the future */ +struct ufs_exynos_handle { + void *private; +}; + +#endif /* _UFS_EXYNOS_IF_H_ */ diff --git a/drivers/scsi/ufs/ufs-exynos.c b/drivers/scsi/ufs/ufs-exynos.c index 440f2af..50437db 100644 --- a/drivers/scsi/ufs/ufs-exynos.c +++ b/drivers/scsi/ufs/ufs-exynos.c @@ -1008,6 +1008,12 @@ static int exynos_ufs_init(struct ufs_hba *hba) goto out; exynos_ufs_specify_phy_time_attr(ufs); exynos_ufs_config_smu(ufs); + + /* init dbg */ + ret = exynos_ufs_init_dbg(ufs->handle); + if (ret) + return ret; + spin_lock_init(&ufs->dbg_lock); return 0; phy_off: @@ -1210,6 +1216,41 @@ static int exynos_ufs_resume(struct ufs_hba *hba, enum ufs_pm_op pm_op) return 0; } +static void exynos_ufs_dbg_register_dump(struct ufs_hba *hba) +{ + struct exynos_ufs *ufs = ufshcd_get_variant(hba); + unsigned long flags; + + spin_lock_irqsave(&ufs->dbg_lock, flags); + if (ufs->under_dump == 0) + ufs->under_dump = 1; + else { + spin_unlock_irqrestore(&ufs->dbg_lock, flags); + goto out; + } + spin_unlock_irqrestore(&ufs->dbg_lock, flags); + + exynos_ufs_dump_info(ufs->handle, hba->dev); + + spin_lock_irqsave(&ufs->dbg_lock, flags); + ufs->under_dump = 0; + spin_unlock_irqrestore(&ufs->dbg_lock, flags); +out: + return; +} + +static void exynos_ufs_cmd_log(struct ufs_hba *hba, + struct scsi_cmnd *cmd, int enter) +{ + struct exynos_ufs *ufs = ufshcd_get_variant(hba); + struct ufs_exynos_handle *handle = ufs->handle; + + if (enter == 1) + exynos_ufs_cmd_log_start(handle, hba, cmd); + else if (enter == 2) + exynos_ufs_cmd_log_end(handle, hba, cmd); +} + static struct ufs_hba_variant_ops ufs_hba_exynos_ops = { .name = "exynos_ufs", .init = exynos_ufs_init, @@ -1221,6 +1262,8 @@ static struct ufs_hba_variant_ops ufs_hba_exynos_ops = { .hibern8_notify = exynos_ufs_hibern8_notify, .suspend = exynos_ufs_suspend, .resume = exynos_ufs_resume, + .dbg_register_dump = exynos_ufs_dbg_register_dump, + .cmd_log = exynos_ufs_cmd_log, }; static int exynos_ufs_probe(struct platform_device *pdev) diff --git a/drivers/scsi/ufs/ufs-exynos.h b/drivers/scsi/ufs/ufs-exynos.h index 76d6e39..14347fb 100644 --- a/drivers/scsi/ufs/ufs-exynos.h +++ b/drivers/scsi/ufs/ufs-exynos.h @@ -8,6 +8,7 @@ #ifndef _UFS_EXYNOS_H_ #define _UFS_EXYNOS_H_ +#include "ufs-exynos-if.h" /* * UNIPRO registers @@ -212,6 +213,10 @@ struct exynos_ufs { #define EXYNOS_UFS_OPT_BROKEN_AUTO_CLK_CTRL BIT(2) #define EXYNOS_UFS_OPT_BROKEN_RX_SEL_IDX BIT(3) #define EXYNOS_UFS_OPT_USE_SW_HIBERN8_TIMER BIT(4) + + struct ufs_exynos_handle *handle; + spinlock_t dbg_lock; + int under_dump; }; #define for_each_ufs_rx_lane(ufs, i) \ @@ -284,4 +289,12 @@ struct exynos_ufs_uic_attr exynos7_uic_attr = { .rx_hs_g3_prep_sync_len_cap = PREP_LEN(0xf), .pa_dbg_option_suite = 0x30103, }; + +/* public function declarations */ +void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct device *dev); +void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle, + struct ufs_hba *hba, struct scsi_cmnd *cmd); +void exynos_ufs_cmd_log_end(struct ufs_exynos_handle *handle, + struct ufs_hba *hba, struct scsi_cmnd *cmd); +int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle); #endif /* _UFS_EXYNOS_H_ */ -- 2.7.4 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 2/2] exynos-ufs: support command history 2020-06-20 6:53 ` [RFC PATCH v1 2/2] exynos-ufs: support command history Kiwoong Kim @ 2020-06-20 15:26 ` kernel test robot 2020-06-20 19:08 ` kernel test robot 2020-06-24 3:10 ` Bart Van Assche 2 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2020-06-20 15:26 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1628 bytes --] Hi Kiwoong, [FYI, it's a private test report for your RFC patch.] [auto build test ERROR on scsi/for-next] [also build test ERROR on mkp-scsi/for-next next-20200618] [cannot apply to v5.8-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200620-150310 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: xtensa-allyesconfig (attached as .config) compiler: xtensa-linux-gcc (GCC) 9.3.0 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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=xtensa If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> xtensa-linux-ld: drivers/scsi/ufs/ufs-exynos-dbg.o:(.data+0x60): multiple definition of `exynos7_uic_attr'; drivers/scsi/ufs/ufs-exynos.o:(.data+0x120): first defined here >> xtensa-linux-ld: drivers/scsi/ufs/ufs-exynos-dbg.o:(.bss+0x7a0): multiple definition of `exynos_ufs_drvs'; drivers/scsi/ufs/ufs-exynos.o:(.data+0x1c0): first defined here --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 64405 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 2/2] exynos-ufs: support command history 2020-06-20 6:53 ` [RFC PATCH v1 2/2] exynos-ufs: support command history Kiwoong Kim 2020-06-20 15:26 ` kernel test robot @ 2020-06-20 19:08 ` kernel test robot 2020-06-24 3:10 ` Bart Van Assche 2 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2020-06-20 19:08 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 1702 bytes --] Hi Kiwoong, [FYI, it's a private test report for your RFC patch.] [auto build test ERROR on scsi/for-next] [also build test ERROR on mkp-scsi/for-next next-20200618] [cannot apply to v5.8-rc1] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200620-150310 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: riscv-allyesconfig (attached as .config) compiler: riscv64-linux-gcc (GCC) 9.3.0 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 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=riscv If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> All errors (new ones prefixed by >>): >> riscv64-linux-ld: drivers/scsi/ufs/ufs-exynos-dbg.o:(.data+0x48): multiple definition of `exynos7_uic_attr'; drivers/scsi/ufs/ufs-exynos.o:ufs-exynos.c:(.data+0x1b8): first defined here riscv64-linux-ld: drivers/scsi/ufs/ufs-exynos-dbg.o: in function `.LANCHOR0': >> ufs-exynos-dbg.c:(.bss+0x770): multiple definition of `exynos_ufs_drvs'; drivers/scsi/ufs/ufs-exynos.o:ufs-exynos.c:(.data+0x178): first defined here --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 65088 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 2/2] exynos-ufs: support command history 2020-06-20 6:53 ` [RFC PATCH v1 2/2] exynos-ufs: support command history Kiwoong Kim 2020-06-20 15:26 ` kernel test robot 2020-06-20 19:08 ` kernel test robot @ 2020-06-24 3:10 ` Bart Van Assche 2020-06-29 8:49 ` Kiwoong Kim 2020-06-29 10:08 ` Kiwoong Kim 2 siblings, 2 replies; 18+ messages in thread From: Bart Van Assche @ 2020-06-24 3:10 UTC (permalink / raw) To: Kiwoong Kim, linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang > +#define EN_PRINT_UFS_LOG 1 Since this macro controls debug code, please make this configurable at runtime, e.g. as a kernel module parameter or by using the dynamic debug mechanism. > +/* Structure for ufs cmd logging */ > +#define MAX_CMD_LOGS 32 Please clarify in the comment above this constant how this constant has been chosen. Is this constant e.g. related to the queue depth? Is this constant per UFS device or is it a maximum for all UFS devices? > +struct cmd_data { > + unsigned int tag; > + unsigned int sct; > + unsigned long lba; > + u64 start_time; > + u64 end_time; > + u64 outstanding_reqs; > + int retries; > + unsigned char op; > +}; Please use data types that explicitly specify the width for members like e.g. the lba (u64 instead of unsigned long). Please also use u8 instead of unsigned char for 'op' since 'op' is not used to store any kind of ASCII character. > +struct ufs_cmd_info { > + u32 total; > + u32 last; > + struct cmd_data data[MAX_CMD_LOGS]; > + struct cmd_data *pdata[32]; /* Currently, 32 slots */ > +}; What are 'slots'? Why 32? > +#define DBG_NUM_OF_HOSTS 1 > +struct ufs_s_dbg_mgr { > + struct ufs_exynos_handle *handle; > + int active; > + u64 first_time; > + u64 time; > + > + /* cmd log */ > + struct ufs_cmd_info cmd_info; > + struct cmd_data cmd_log; /* temp buffer to put */ > + spinlock_t cmd_lock; > +}; Please add a comment above this structure that explains the role of this data structure. > +static struct ufs_s_dbg_mgr ufs_s_dbg[DBG_NUM_OF_HOSTS]; A static array? That's suspicious. Should that array perhaps be a member of another UFS data structure, e.g. the UFS host or device data structure? > +static int ufs_s_dbg_mgr_idx; What does this variable represent? > + for (i = 0 ; i < max ; i++, data++) { > + dev_err(dev, ": 0x%02x, %02d, 0x%08lx, 0x%04x, %d, %llu, %llu, 0x%llx %s", > + data->op, > + data->tag, > + data->lba, > + data->sct, > + data->retries, > + data->start_time, > + data->end_time, > + data->outstanding_reqs, > + ((last == i) ? "<--" : " ")); Please follow the same coding style as elsewhere in the Linux kernel and specify multiple arguments per line (up to the current column limit). Please also align the arguments either with the opening parentheses or indent these by one tab as requested in the Linux kernel coding style document. > +/* > + * EXTERNAL FUNCTIONS > + * > + * There are two classes that are to initialize data structures for debug > + * and to define actual behavior. > + */ > +void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct device *dev) > +{ > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > + > + if (mgr->active == 0) > + goto out; > + > + mgr->time = cpu_clock(raw_smp_processor_id()); > + > +#if EN_PRINT_UFS_LOG > + ufs_s_print_cmd_log(mgr, dev); > +#endif > + > + if (mgr->first_time == 0ULL) > + mgr->first_time = mgr->time; > +out: > + return; > +} Using cpu_clock() without storing the CPU on which it has been sampled seems wrong to me. Is higher accuracy than a single jiffy required? If not, how about using 'jiffies' instead? From clock.h: /* * As outlined in clock.c, provides a fast, high resolution, nanosecond * time source that is monotonic per cpu argument and has bounded drift * between cpus. * * ######################### BIG FAT WARNING ########################## * # when comparing cpu_clock(i) to cpu_clock(j) for i != j, time can # * # go backwards !! # * #################################################################### */ > +void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle, > + struct ufs_hba *hba, struct scsi_cmnd *cmd) > +{ > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > + int cpu = raw_smp_processor_id(); > + struct cmd_data *cmd_log = &mgr->cmd_log; /* temp buffer to put */ > + unsigned long lba = (cmd->cmnd[2] << 24) | > + (cmd->cmnd[3] << 16) | > + (cmd->cmnd[4] << 8) | > + (cmd->cmnd[5] << 0); > + unsigned int sct = (cmd->cmnd[7] << 8) | > + (cmd->cmnd[8] << 0); Aargh! SCSI command parsing ... Has it been considered to use blk_rq_pos(cmd->req) and blk_rq_bytes(cmd->req) instead? > +int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle) > +{ > + struct ufs_s_dbg_mgr *mgr; > + > + if (ufs_s_dbg_mgr_idx >= DBG_NUM_OF_HOSTS) > + return -1; > + > + mgr = &ufs_s_dbg[ufs_s_dbg_mgr_idx++]; > + handle->private = (void *)mgr; > + mgr->handle = handle; > + mgr->active = 1; Can the '(void *)' cast above be left out? > +#define UFS_VS_MMIO_VER 1 What does this constant represent? Please add a comment. Thanks, Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 2/2] exynos-ufs: support command history 2020-06-24 3:10 ` Bart Van Assche @ 2020-06-29 8:49 ` Kiwoong Kim 2020-06-29 10:08 ` Kiwoong Kim 1 sibling, 0 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-29 8:49 UTC (permalink / raw) To: 'Bart Van Assche', linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang > > +#define EN_PRINT_UFS_LOG 1 > > Since this macro controls debug code, please make this configurable at > runtime, e.g. as a kernel module parameter or by using the dynamic debug > mechanism. > > > +/* Structure for ufs cmd logging */ > > +#define MAX_CMD_LOGS 32 Got it. > > Please clarify in the comment above this constant how this constant has > been chosen. Is this constant e.g. related to the queue depth? Is this > constant per UFS device or is it a maximum for all UFS devices? > > > +struct cmd_data { > > + unsigned int tag; > > + unsigned int sct; > > + unsigned long lba; > > + u64 start_time; > > + u64 end_time; > > + u64 outstanding_reqs; > > + int retries; > > + unsigned char op; > > +}; > > Please use data types that explicitly specify the width for members like > e.g. the lba (u64 instead of unsigned long). Please also use u8 instead > of unsigned char for ' op' since 'op' is not used to store any kind of > ASCII character. Got it. > > > +struct ufs_cmd_info { > > + u32 total; > > + u32 last; > > + struct cmd_data data[MAX_CMD_LOGS]; > > + struct cmd_data *pdata[32]; /* Currently, 32 slots */ > > +}; > > What are 'slots'? Why 32? I meant tag number and assumed that CAP.NUTRS be 32. With 32, you can see all the command context with full outstanding situations. Of course, there is a possibility that the value increases in the next UFS versions. So, to run automatically, yes, I should have made this get CAP.NUTRS. However, to do that, I have to add another call after enabling host. > > > +#define DBG_NUM_OF_HOSTS 1 > > +struct ufs_s_dbg_mgr { > > + struct ufs_exynos_handle *handle; > > + int active; > > + u64 first_time; > > + u64 time; > > + > > + /* cmd log */ > > + struct ufs_cmd_info cmd_info; > > + struct cmd_data cmd_log; /* temp buffer to put */ > > + spinlock_t cmd_lock; > > +}; > > Please add a comment above this structure that explains the role of this > data structure. Got it. > > > +static struct ufs_s_dbg_mgr ufs_s_dbg[DBG_NUM_OF_HOSTS]; > > A static array? That's suspicious. Should that array perhaps be a member > of another UFS data structure, e.g. the UFS host or device data structure? Got it. > > > +static int ufs_s_dbg_mgr_idx; > > What does this variable represent? I considered it again and will remove. > > > + for (i = 0 ; i < max ; i++, data++) { > > + dev_err(dev, ": 0x%02x, %02d, 0x%08lx, > 0x%04x, %d, %llu, %llu, 0x%llx %s", > > + data->op, > > + data->tag, > > + data->lba, > > + data->sct, > > + data->retries, > > + data->start_time, > > + data->end_time, > > + data->outstanding_reqs, > > + ((last == i) ? "<--" : " ")); > > Please follow the same coding style as elsewhere in the Linux kernel and > specify multiple arguments per line (up to the current column limit). > Please also align the arguments either with the opening parentheses or > indent these by one tab as requested in the Linux kernel coding style > document. Got it. > > > +/* > > + * EXTERNAL FUNCTIONS > > + * > > + * There are two classes that are to initialize data structures for > debug > > + * and to define actual behavior. > > + */ > > +void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct > device *dev) > > +{ > > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > > + > > + if (mgr->active == 0) > > + goto out; > > + > > + mgr->time = cpu_clock(raw_smp_processor_id()); > > + > > +#if EN_PRINT_UFS_LOG > > + ufs_s_print_cmd_log(mgr, dev); > > +#endif > > + > > + if (mgr->first_time == 0ULL) > > + mgr->first_time = mgr->time; > > +out: > > + return; > > +} > > Using cpu_clock() without storing the CPU on which it has been sampled > seems wrong to me. Is higher accuracy than a single jiffy required? If > not, how about using 'jiffies' instead? From clock.h: > I think jiffies isn't proper for the original purpose. > /* > * As outlined in clock.c, provides a fast, high resolution, nanosecond > * time source that is monotonic per cpu argument and has bounded drift > * between cpus. > * > * ######################### BIG FAT WARNING ########################## > * # when comparing cpu_clock(i) to cpu_clock(j) for i != j, time can # > * # go backwards !! # > * #################################################################### > */ > > > +void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle, > > + struct ufs_hba *hba, struct scsi_cmnd *cmd) > > +{ > > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > > + int cpu = raw_smp_processor_id(); > > + struct cmd_data *cmd_log = &mgr->cmd_log; /* temp buffer to put > */ > > + unsigned long lba = (cmd->cmnd[2] << 24) | > > + (cmd->cmnd[3] << 16) | > > + (cmd->cmnd[4] << 8) | > > + (cmd->cmnd[5] << 0); > > + unsigned int sct = (cmd->cmnd[7] << 8) | > > + (cmd->cmnd[8] << 0); > > Aargh! SCSI command parsing ... Has it been considered to use > blk_rq_pos(cmd->req) and blk_rq_bytes(cmd->req) instead? Sure and I have recognized those things for years. UFS depends on SCSI and regarding to actual usages. Thus, I don't feel that sort of parsing unless there is another macro to parse SCSI commands. As I know, nothing exits. And blk_rq_pos doesn't represent actual lba. That represents in-partition addressing. With this, I have to refer to the start address of partitions and I don’t think that isn't suitable in UFS driver. > > > +int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle) > > +{ > > + struct ufs_s_dbg_mgr *mgr; > > + > > + if (ufs_s_dbg_mgr_idx >= DBG_NUM_OF_HOSTS) > > + return -1; > > + > > + mgr = &ufs_s_dbg[ufs_s_dbg_mgr_idx++]; > > + handle->private = (void *)mgr; > > + mgr->handle = handle; > > + mgr->active = 1; > > Can the '(void *)' cast above be left out? I didn't want to share the details of this structure with ufs-exynos.c. I don’t feel that it's better. > > > +#define UFS_VS_MMIO_VER 1 > > What does this constant represent? Please add a comment. I considered it again and will remove. > > Thanks, > > Bart. Thank you for your comments even if this is a RFS patch. Thanks. Kiwoong Kim ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 2/2] exynos-ufs: support command history 2020-06-24 3:10 ` Bart Van Assche 2020-06-29 8:49 ` Kiwoong Kim @ 2020-06-29 10:08 ` Kiwoong Kim 1 sibling, 0 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-29 10:08 UTC (permalink / raw) To: linux-scsi > > +#define EN_PRINT_UFS_LOG 1 > > Since this macro controls debug code, please make this configurable at > runtime, e.g. as a kernel module parameter or by using the dynamic debug > mechanism. Got it. > > > +/* Structure for ufs cmd logging */ > > +#define MAX_CMD_LOGS 32 > > Please clarify in the comment above this constant how this constant has > been chosen. Is this constant e.g. related to the queue depth? Is this > constant per UFS device or is it a maximum for all UFS devices? > > > +struct cmd_data { > > + unsigned int tag; > > + unsigned int sct; > > + unsigned long lba; > > + u64 start_time; > > + u64 end_time; > > + u64 outstanding_reqs; > > + int retries; > > + unsigned char op; > > +}; > > Please use data types that explicitly specify the width for members like > e.g. the lba (u64 instead of unsigned long). Please also use u8 instead of > unsigned char for 'op' since 'op' is not used to store any kind of ASCII > character. Got it. > > > +struct ufs_cmd_info { > > + u32 total; > > + u32 last; > > + struct cmd_data data[MAX_CMD_LOGS]; > > + struct cmd_data *pdata[32]; /* Currently, 32 slots */ > > +}; > > What are 'slots'? Why 32? I meant tag number and assumed that CAP.NUTRS be 32. With 32, you can see all the command context with full outstanding situations. Of course, there is a possibility that the value increases in the next UFS versions. So, to run automatically, yes, I should have made this get CAP.NUTRS. However, to do that, I have to add another call after enabling host. > > > +#define DBG_NUM_OF_HOSTS 1 > > +struct ufs_s_dbg_mgr { > > + struct ufs_exynos_handle *handle; > > + int active; > > + u64 first_time; > > + u64 time; > > + > > + /* cmd log */ > > + struct ufs_cmd_info cmd_info; > > + struct cmd_data cmd_log; /* temp buffer to put */ > > + spinlock_t cmd_lock; > > +}; > > Please add a comment above this structure that explains the role of this > data structure. Got it. > > > +static struct ufs_s_dbg_mgr ufs_s_dbg[DBG_NUM_OF_HOSTS]; > > A static array? That's suspicious. Should that array perhaps be a member > of another UFS data structure, e.g. the UFS host or device data structure? > > > +static int ufs_s_dbg_mgr_idx; I considered it again and will remove. > > What does this variable represent? > > > + for (i = 0 ; i < max ; i++, data++) { > > + dev_err(dev, ": 0x%02x, %02d, 0x%08lx, > 0x%04x, %d, %llu, %llu, 0x%llx %s", > > + data->op, > > + data->tag, > > + data->lba, > > + data->sct, > > + data->retries, > > + data->start_time, > > + data->end_time, > > + data->outstanding_reqs, > > + ((last == i) ? "<--" : " ")); > > Please follow the same coding style as elsewhere in the Linux kernel and > specify multiple arguments per line (up to the current column limit). > Please also align the arguments either with the opening parentheses or > indent these by one tab as requested in the Linux kernel coding style > document. > Got it. > > +/* > > + * EXTERNAL FUNCTIONS > > + * > > + * There are two classes that are to initialize data structures for > > +debug > > + * and to define actual behavior. > > + */ > > +void exynos_ufs_dump_info(struct ufs_exynos_handle *handle, struct > > +device *dev) { > > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > > + > > + if (mgr->active == 0) > > + goto out; > > + > > + mgr->time = cpu_clock(raw_smp_processor_id()); > > + > > +#if EN_PRINT_UFS_LOG > > + ufs_s_print_cmd_log(mgr, dev); > > +#endif > > + > > + if (mgr->first_time == 0ULL) > > + mgr->first_time = mgr->time; > > +out: > > + return; > > +} > > Using cpu_clock() without storing the CPU on which it has been sampled > seems wrong to me. Is higher accuracy than a single jiffy required? If not, > how about using 'jiffies' instead? From clock.h: I think jiffies isn't proper for the original purpose. > > /* > * As outlined in clock.c, provides a fast, high resolution, nanosecond > * time source that is monotonic per cpu argument and has bounded drift > * between cpus. > * > * ######################### BIG FAT WARNING ########################## > * # when comparing cpu_clock(i) to cpu_clock(j) for i != j, time can # > * # go backwards !! # > * #################################################################### > */ > > > +void exynos_ufs_cmd_log_start(struct ufs_exynos_handle *handle, > > + struct ufs_hba *hba, struct scsi_cmnd *cmd) { > > + struct ufs_s_dbg_mgr *mgr = (struct ufs_s_dbg_mgr *)handle->private; > > + int cpu = raw_smp_processor_id(); > > + struct cmd_data *cmd_log = &mgr->cmd_log; /* temp buffer to put > */ > > + unsigned long lba = (cmd->cmnd[2] << 24) | > > + (cmd->cmnd[3] << 16) | > > + (cmd->cmnd[4] << 8) | > > + (cmd->cmnd[5] << 0); > > + unsigned int sct = (cmd->cmnd[7] << 8) | > > + (cmd->cmnd[8] << 0); > > Aargh! SCSI command parsing ... Has it been considered to use > blk_rq_pos(cmd->req) and blk_rq_bytes(cmd->req) instead? Sure and I have recognized those things for years. UFS depends on SCSI and regarding to actual usages. Thus, I don't feel that sort of parsing unless there is another macro to parse SCSI commands. As I know, nothing exits. And blk_rq_pos doesn't represent actual lba. That represents in-partition addressing. With this, I have to refer to the start address of partitions and I don’t think that isn't suitable in UFS driver. > > > +int exynos_ufs_init_dbg(struct ufs_exynos_handle *handle) { > > + struct ufs_s_dbg_mgr *mgr; > > + > > + if (ufs_s_dbg_mgr_idx >= DBG_NUM_OF_HOSTS) > > + return -1; > > + > > + mgr = &ufs_s_dbg[ufs_s_dbg_mgr_idx++]; > > + handle->private = (void *)mgr; > > + mgr->handle = handle; > > + mgr->active = 1; > > Can the '(void *)' cast above be left out? I didn't want to share the details of this structure with ufs-exynos.c. I don’t feel that it's better. > > > +#define UFS_VS_MMIO_VER 1 > > What does this constant represent? Please add a comment. I considered it again and will remove. > > Thanks, > > Bart. Thank you for your comments even if this is a RFS patch. Thanks. Kiwoong Kim ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-20 6:53 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Kiwoong Kim [not found] ` <CGME20200620070047epcas2p37229d52d479df9f64ee4fc14f469acf9@epcas2p3.samsung.com> @ 2020-06-20 16:33 ` Bart Van Assche 2020-06-23 2:28 ` Kiwoong Kim 2020-06-22 11:35 ` Dan Carpenter 2020-06-25 14:05 ` Stanley Chu 3 siblings, 1 reply; 18+ messages in thread From: Bart Van Assche @ 2020-06-20 16:33 UTC (permalink / raw) To: Kiwoong Kim, linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, Christoph Hellwig On 2020-06-19 23:53, Kiwoong Kim wrote: > Some SoC specific might need command history for > various reasons, such as stacking command contexts > in system memory to check for debugging in the future > or scaling some DVFS knobs to boost IO throughput. > > What you would do with the information could be > variant per SoC vendor. Isn't this something that should be done in an I/O scheduler instead of in a SCSI LLD? I don't like the idea behind this patch series at all. Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-20 16:33 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Bart Van Assche @ 2020-06-23 2:28 ` Kiwoong Kim 2020-06-24 2:53 ` Bart Van Assche 0 siblings, 1 reply; 18+ messages in thread From: Kiwoong Kim @ 2020-06-23 2:28 UTC (permalink / raw) To: 'Bart Van Assche', linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, 'Christoph Hellwig' > On 2020-06-19 23:53, Kiwoong Kim wrote: > > Some SoC specific might need command history for various reasons, such > > as stacking command contexts in system memory to check for debugging > > in the future or scaling some DVFS knobs to boost IO throughput. > > > > What you would do with the information could be variant per SoC > > vendor. > > Isn't this something that should be done in an I/O scheduler instead of in > a SCSI LLD? I don't like the idea behind this patch series at all. > > Bart. If you could get the information, Many would exploit it for their respective purposes. But, it's important for the information to contain accurate timestamps when the driver hooks it, if you're trying to figure out something wrong. As for scaling DVFS knobs to boost UFS throughput, locating in the ufs driver is more Beneficial because SoC vendors have their own power domains and thus lead to make different way of what to scale up to boost. If it's populated in block layer, there is no way to introduce boosting per SoC. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-23 2:28 ` Kiwoong Kim @ 2020-06-24 2:53 ` Bart Van Assche 0 siblings, 0 replies; 18+ messages in thread From: Bart Van Assche @ 2020-06-24 2:53 UTC (permalink / raw) To: Kiwoong Kim, linux-scsi, alim.akhtar, avri.altman, jejb, martin.petersen, beanhuo, asutoshd, cang, 'Christoph Hellwig' On 2020-06-22 19:28, Kiwoong Kim wrote: > If you could get the information, Many would exploit it for their respective purposes. > But, it's important for the information to contain accurate timestamps when the driver > hooks it, if you're trying to figure out something wrong. > > As for scaling DVFS knobs to boost UFS throughput, locating in the ufs driver is more > Beneficial because SoC vendors have their own power domains and thus lead to make > different way of what to scale up to boost. If it's populated in block layer, there is > no way to introduce boosting per SoC. Thanks for the clarification. Unfortunately we do not yet have an API for sharing information between I/O schedulers and block drivers ... Bart. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-20 6:53 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Kiwoong Kim @ 2020-06-22 11:35 ` Dan Carpenter 2020-06-20 16:33 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Bart Van Assche ` (2 subsequent siblings) 3 siblings, 0 replies; 18+ messages in thread From: Dan Carpenter @ 2020-06-22 11:35 UTC (permalink / raw) To: kbuild [-- Attachment #1: Type: text/plain, Size: 8382 bytes --] Hi Kiwoong, url: https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200620-150310 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: x86_64-randconfig-m001-20200620 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/scsi/ufs/ufshcd.c:2548 ufshcd_queuecommand() warn: variable dereferenced before check 'cmd' (see line 2475) # https://github.com/0day-ci/linux/commit/cff1afd5a0773e6da9106e953721996a56a9332c git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout cff1afd5a0773e6da9106e953721996a56a9332c vim +/cmd +2548 drivers/scsi/ufs/ufshcd.c 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2465 static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2466 { 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2467 struct ufshcd_lrb *lrbp; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2468 struct ufs_hba *hba; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2469 unsigned long flags; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2470 int tag; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2471 int err = 0; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2472 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2473 hba = shost_priv(host); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2474 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 @2475 tag = cmd->request->tag; ^^^^^^^^^^^^^^^^^ Dereference. 14497328b6a628 Yaniv Gardi 2016-02-01 2476 if (!ufshcd_valid_tag(hba, tag)) { 14497328b6a628 Yaniv Gardi 2016-02-01 2477 dev_err(hba->dev, 14497328b6a628 Yaniv Gardi 2016-02-01 2478 "%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p", 14497328b6a628 Yaniv Gardi 2016-02-01 2479 __func__, tag, cmd, cmd->request); 14497328b6a628 Yaniv Gardi 2016-02-01 2480 BUG(); 14497328b6a628 Yaniv Gardi 2016-02-01 2481 } 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2482 a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2483 if (!down_read_trylock(&hba->clk_scaling_lock)) a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2484 return SCSI_MLQUEUE_HOST_BUSY; a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2485 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2486 spin_lock_irqsave(hba->host->host_lock, flags); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2487 switch (hba->ufshcd_state) { 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2488 case UFSHCD_STATE_OPERATIONAL: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2489 break; 141f81651037ea Zang Leigang 2016-11-16 2490 case UFSHCD_STATE_EH_SCHEDULED: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2491 case UFSHCD_STATE_RESET: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2492 err = SCSI_MLQUEUE_HOST_BUSY; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2493 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2494 case UFSHCD_STATE_ERROR: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2495 set_host_byte(cmd, DID_ERROR); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2496 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2497 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2498 default: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2499 dev_WARN_ONCE(hba->dev, 1, "%s: invalid state %d\n", 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2500 __func__, hba->ufshcd_state); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2501 set_host_byte(cmd, DID_BAD_TARGET); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2502 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2503 goto out_unlock; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2504 } 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2505 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2506 /* if error handling is in progress, don't issue commands */ 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2507 if (ufshcd_eh_in_progress(hba)) { 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2508 set_host_byte(cmd, DID_ERROR); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2509 cmd->scsi_done(cmd); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2510 goto out_unlock; 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2511 } 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2512 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2513 7fabb77b3aa016 Gilad Broner 2017-02-03 2514 hba->req_abort_count = 0; 7fabb77b3aa016 Gilad Broner 2017-02-03 2515 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2516 err = ufshcd_hold(hba, true); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2517 if (err) { 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2518 err = SCSI_MLQUEUE_HOST_BUSY; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2519 goto out; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2520 } 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2521 WARN_ON(hba->clk_gating.state != CLKS_ON); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2522 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2523 lrbp = &hba->lrb[tag]; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2524 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2525 WARN_ON(lrbp->cmd); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2526 lrbp->cmd = cmd; 09a5a24ff36f90 Avri Altman 2018-11-22 2527 lrbp->sense_bufflen = UFS_SENSE_SIZE; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2528 lrbp->sense_buffer = cmd->sense_buffer; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2529 lrbp->task_tag = tag; 0ce147d48a3e33 Subhash Jadavani 2014-09-25 2530 lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun); b852190e589abe Yaniv Gardi 2015-05-17 2531 lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false; e0b299e36004f5 Gilad Broner 2017-02-03 2532 lrbp->req_abort_skip = false; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2533 300bb13f5c7b1d Joao Pinto 2016-05-11 2534 ufshcd_comp_scsi_upiu(hba, lrbp); 300bb13f5c7b1d Joao Pinto 2016-05-11 2535 75b1cc4ad63afa Kiwoong Kim 2016-11-22 2536 err = ufshcd_map_sg(hba, lrbp); 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2537 if (err) { 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2538 lrbp->cmd = NULL; 17c7d35f141ef6 Can Guo 2019-12-05 2539 ufshcd_release(hba); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2540 goto out; 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2541 } ad1a1b9cd67a4b Gilad Broner 2016-10-17 2542 /* Make sure descriptors are ready before ringing the doorbell */ ad1a1b9cd67a4b Gilad Broner 2016-10-17 2543 wmb(); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2544 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2545 /* issue command to the controller */ 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2546 spin_lock_irqsave(hba->host->host_lock, flags); 5905d4648e7ec2 Bart Van Assche 2020-01-22 2547 ufshcd_vops_setup_xfer_req(hba, tag, true); cff1afd5a0773e Kiwoong Kim 2020-06-20 @2548 if (cmd) ^^^ If "cmd" is NULL then we would have already crashed. cff1afd5a0773e Kiwoong Kim 2020-06-20 2549 ufshcd_vops_cmd_log(hba, cmd, 1); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2550 ufshcd_send_command(hba, tag); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2551 out_unlock: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2552 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2553 out: a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2554 up_read(&hba->clk_scaling_lock); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2555 return err; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2556 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 32367 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information @ 2020-06-22 11:35 ` Dan Carpenter 0 siblings, 0 replies; 18+ messages in thread From: Dan Carpenter @ 2020-06-22 11:35 UTC (permalink / raw) To: kbuild-all [-- Attachment #1: Type: text/plain, Size: 8382 bytes --] Hi Kiwoong, url: https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200620-150310 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next config: x86_64-randconfig-m001-20200620 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/scsi/ufs/ufshcd.c:2548 ufshcd_queuecommand() warn: variable dereferenced before check 'cmd' (see line 2475) # https://github.com/0day-ci/linux/commit/cff1afd5a0773e6da9106e953721996a56a9332c git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout cff1afd5a0773e6da9106e953721996a56a9332c vim +/cmd +2548 drivers/scsi/ufs/ufshcd.c 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2465 static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2466 { 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2467 struct ufshcd_lrb *lrbp; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2468 struct ufs_hba *hba; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2469 unsigned long flags; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2470 int tag; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2471 int err = 0; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2472 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2473 hba = shost_priv(host); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2474 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 @2475 tag = cmd->request->tag; ^^^^^^^^^^^^^^^^^ Dereference. 14497328b6a628 Yaniv Gardi 2016-02-01 2476 if (!ufshcd_valid_tag(hba, tag)) { 14497328b6a628 Yaniv Gardi 2016-02-01 2477 dev_err(hba->dev, 14497328b6a628 Yaniv Gardi 2016-02-01 2478 "%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p", 14497328b6a628 Yaniv Gardi 2016-02-01 2479 __func__, tag, cmd, cmd->request); 14497328b6a628 Yaniv Gardi 2016-02-01 2480 BUG(); 14497328b6a628 Yaniv Gardi 2016-02-01 2481 } 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2482 a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2483 if (!down_read_trylock(&hba->clk_scaling_lock)) a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2484 return SCSI_MLQUEUE_HOST_BUSY; a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2485 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2486 spin_lock_irqsave(hba->host->host_lock, flags); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2487 switch (hba->ufshcd_state) { 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2488 case UFSHCD_STATE_OPERATIONAL: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2489 break; 141f81651037ea Zang Leigang 2016-11-16 2490 case UFSHCD_STATE_EH_SCHEDULED: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2491 case UFSHCD_STATE_RESET: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2492 err = SCSI_MLQUEUE_HOST_BUSY; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2493 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2494 case UFSHCD_STATE_ERROR: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2495 set_host_byte(cmd, DID_ERROR); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2496 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2497 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2498 default: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2499 dev_WARN_ONCE(hba->dev, 1, "%s: invalid state %d\n", 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2500 __func__, hba->ufshcd_state); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2501 set_host_byte(cmd, DID_BAD_TARGET); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2502 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2503 goto out_unlock; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2504 } 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2505 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2506 /* if error handling is in progress, don't issue commands */ 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2507 if (ufshcd_eh_in_progress(hba)) { 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2508 set_host_byte(cmd, DID_ERROR); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2509 cmd->scsi_done(cmd); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2510 goto out_unlock; 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2511 } 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2512 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2513 7fabb77b3aa016 Gilad Broner 2017-02-03 2514 hba->req_abort_count = 0; 7fabb77b3aa016 Gilad Broner 2017-02-03 2515 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2516 err = ufshcd_hold(hba, true); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2517 if (err) { 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2518 err = SCSI_MLQUEUE_HOST_BUSY; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2519 goto out; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2520 } 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2521 WARN_ON(hba->clk_gating.state != CLKS_ON); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2522 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2523 lrbp = &hba->lrb[tag]; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2524 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2525 WARN_ON(lrbp->cmd); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2526 lrbp->cmd = cmd; 09a5a24ff36f90 Avri Altman 2018-11-22 2527 lrbp->sense_bufflen = UFS_SENSE_SIZE; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2528 lrbp->sense_buffer = cmd->sense_buffer; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2529 lrbp->task_tag = tag; 0ce147d48a3e33 Subhash Jadavani 2014-09-25 2530 lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun); b852190e589abe Yaniv Gardi 2015-05-17 2531 lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false; e0b299e36004f5 Gilad Broner 2017-02-03 2532 lrbp->req_abort_skip = false; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2533 300bb13f5c7b1d Joao Pinto 2016-05-11 2534 ufshcd_comp_scsi_upiu(hba, lrbp); 300bb13f5c7b1d Joao Pinto 2016-05-11 2535 75b1cc4ad63afa Kiwoong Kim 2016-11-22 2536 err = ufshcd_map_sg(hba, lrbp); 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2537 if (err) { 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2538 lrbp->cmd = NULL; 17c7d35f141ef6 Can Guo 2019-12-05 2539 ufshcd_release(hba); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2540 goto out; 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2541 } ad1a1b9cd67a4b Gilad Broner 2016-10-17 2542 /* Make sure descriptors are ready before ringing the doorbell */ ad1a1b9cd67a4b Gilad Broner 2016-10-17 2543 wmb(); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2544 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2545 /* issue command to the controller */ 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2546 spin_lock_irqsave(hba->host->host_lock, flags); 5905d4648e7ec2 Bart Van Assche 2020-01-22 2547 ufshcd_vops_setup_xfer_req(hba, tag, true); cff1afd5a0773e Kiwoong Kim 2020-06-20 @2548 if (cmd) ^^^ If "cmd" is NULL then we would have already crashed. cff1afd5a0773e Kiwoong Kim 2020-06-20 2549 ufshcd_vops_cmd_log(hba, cmd, 1); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2550 ufshcd_send_command(hba, tag); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2551 out_unlock: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2552 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2553 out: a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2554 up_read(&hba->clk_scaling_lock); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2555 return err; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2556 } --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 32367 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-20 6:53 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Kiwoong Kim ` (2 preceding siblings ...) 2020-06-22 11:35 ` Dan Carpenter @ 2020-06-25 14:05 ` Stanley Chu 2020-06-26 11:42 ` Kiwoong Kim 3 siblings, 1 reply; 18+ messages in thread From: Stanley Chu @ 2020-06-25 14:05 UTC (permalink / raw) To: Kiwoong Kim Cc: linux-scsi, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo (beanhuo), Asutosh Das, cang, bvanassche Hi Kiwoong, Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月20日 週六 下午3:00寫道: > > Some SoC specific might need command history for > various reasons, such as stacking command contexts > in system memory to check for debugging in the future > or scaling some DVFS knobs to boost IO throughput. > > What you would do with the information could be > variant per SoC vendor. > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > --- > drivers/scsi/ufs/ufshcd.c | 4 ++++ > drivers/scsi/ufs/ufshcd.h | 8 ++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > index 52abe82..0eae3ce 100644 > --- a/drivers/scsi/ufs/ufshcd.c > +++ b/drivers/scsi/ufs/ufshcd.c > @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) > /* issue command to the controller */ > spin_lock_irqsave(hba->host->host_lock, flags); > ufshcd_vops_setup_xfer_req(hba, tag, true); > + if (cmd) > + ufshcd_vops_cmd_log(hba, cmd, 1); > ufshcd_send_command(hba, tag); > out_unlock: > spin_unlock_irqrestore(hba->host->host_lock, flags); > @@ -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct ufs_hba *hba, > /* Mark completed command as NULL in LRB */ > lrbp->cmd = NULL; > lrbp->compl_time_stamp = ktime_get(); > + ufshcd_vops_cmd_log(hba, cmd, 2); > + > /* Do not touch lrbp after scsi done */ > cmd->scsi_done(cmd); > __ufshcd_release(hba); If your cmd_log vop callbacks are only existed in "ufshcd_queuecommand" and "ufshcd_transfer_req_compl", perhaps you could re-use "ufshcd_vops_setup_xfer_req()" and an added "ufshcd_vops_compl_req()" instead of a brand new "ufshcd_vops_cmd_log()" ? Thanks, Stanley Chu ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-25 14:05 ` Stanley Chu @ 2020-06-26 11:42 ` Kiwoong Kim 2020-06-26 12:29 ` Stanley Chu 0 siblings, 1 reply; 18+ messages in thread From: Kiwoong Kim @ 2020-06-26 11:42 UTC (permalink / raw) To: 'Stanley Chu' Cc: linux-scsi, 'Alim Akhtar', 'Avri Altman', 'James E.J. Bottomley', 'Martin K. Petersen', 'Bean Huo (beanhuo)', 'Asutosh Das', cang, bvanassche > Hi Kiwoong, > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月20日 週六 下午3:00寫道: > > > > Some SoC specific might need command history for various reasons, such > > as stacking command contexts in system memory to check for debugging > > in the future or scaling some DVFS knobs to boost IO throughput. > > > > What you would do with the information could be variant per SoC > > vendor. > > > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > > --- > > drivers/scsi/ufs/ufshcd.c | 4 ++++ > > drivers/scsi/ufs/ufshcd.h | 8 ++++++++ > > 2 files changed, 12 insertions(+) > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > index 52abe82..0eae3ce 100644 > > --- a/drivers/scsi/ufs/ufshcd.c > > +++ b/drivers/scsi/ufs/ufshcd.c > > @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct Scsi_Host > *host, struct scsi_cmnd *cmd) > > /* issue command to the controller */ > > spin_lock_irqsave(hba->host->host_lock, flags); > > ufshcd_vops_setup_xfer_req(hba, tag, true); > > + if (cmd) > > + ufshcd_vops_cmd_log(hba, cmd, 1); > > ufshcd_send_command(hba, tag); > > out_unlock: > > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ > > -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct > ufs_hba *hba, > > /* Mark completed command as NULL in LRB */ > > lrbp->cmd = NULL; > > lrbp->compl_time_stamp = ktime_get(); > > + ufshcd_vops_cmd_log(hba, cmd, 2); > > + > > /* Do not touch lrbp after scsi done */ > > cmd->scsi_done(cmd); > > __ufshcd_release(hba); > > If your cmd_log vop callbacks are only existed in "ufshcd_queuecommand" > and "ufshcd_transfer_req_compl", perhaps you could re-use > "ufshcd_vops_setup_xfer_req()" and an added "ufshcd_vops_compl_req()" > instead of a brand new "ufshcd_vops_cmd_log()" ? > > Thanks, > Stanley Chu Currently, ufshcd_vops_setup_xfer_req doesn't get scsi_cmnd variable. Actually, when introduced this callback first, I was willing to make it do that but someone gave me another idea. Then do you agree to change argument set of the function? And I can't find ufshcd_vops_compl_req in 5.9/scsi-queue. Could you let me know where to find? Thank you for your opinions. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-26 11:42 ` Kiwoong Kim @ 2020-06-26 12:29 ` Stanley Chu 2020-06-29 7:01 ` Kiwoong Kim 2020-06-29 7:11 ` Kiwoong Kim 0 siblings, 2 replies; 18+ messages in thread From: Stanley Chu @ 2020-06-26 12:29 UTC (permalink / raw) To: Kiwoong Kim Cc: linux-scsi, Alim Akhtar, Avri Altman, James E.J. Bottomley, Martin K. Petersen, Bean Huo (beanhuo), Asutosh Das, cang, bvanassche Hi Kiwoong, Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月26日 週五 下午7:42寫道: > > > Hi Kiwoong, > > > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月20日 週六 下午3:00寫道: > > > > > > Some SoC specific might need command history for various reasons, such > > > as stacking command contexts in system memory to check for debugging > > > in the future or scaling some DVFS knobs to boost IO throughput. > > > > > > What you would do with the information could be variant per SoC > > > vendor. > > > > > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > > > --- > > > drivers/scsi/ufs/ufshcd.c | 4 ++++ > > > drivers/scsi/ufs/ufshcd.h | 8 ++++++++ > > > 2 files changed, 12 insertions(+) > > > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > > index 52abe82..0eae3ce 100644 > > > --- a/drivers/scsi/ufs/ufshcd.c > > > +++ b/drivers/scsi/ufs/ufshcd.c > > > @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct Scsi_Host > > *host, struct scsi_cmnd *cmd) > > > /* issue command to the controller */ > > > spin_lock_irqsave(hba->host->host_lock, flags); > > > ufshcd_vops_setup_xfer_req(hba, tag, true); > > > + if (cmd) > > > + ufshcd_vops_cmd_log(hba, cmd, 1); > > > ufshcd_send_command(hba, tag); > > > out_unlock: > > > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ > > > -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct > > ufs_hba *hba, > > > /* Mark completed command as NULL in LRB */ > > > lrbp->cmd = NULL; > > > lrbp->compl_time_stamp = ktime_get(); > > > + ufshcd_vops_cmd_log(hba, cmd, 2); > > > + > > > /* Do not touch lrbp after scsi done */ > > > cmd->scsi_done(cmd); > > > __ufshcd_release(hba); > > > > If your cmd_log vop callbacks are only existed in "ufshcd_queuecommand" > > and "ufshcd_transfer_req_compl", perhaps you could re-use > > "ufshcd_vops_setup_xfer_req()" and an added "ufshcd_vops_compl_req()" > > instead of a brand new "ufshcd_vops_cmd_log()" ? > > > > Thanks, > > Stanley Chu > > Currently, ufshcd_vops_setup_xfer_req doesn't get scsi_cmnd variable. You could get scsi_cmnd by hba->lrb[tag].cmd if is_scsi_cmd is true in your ufshcd_vops_setup_xfer_req vendor implementation. > Actually, when introduced this callback first, I was willing to make it do that > but someone gave me another idea. Then do you agree to change argument set of the function? > > And I can't find ufshcd_vops_compl_req in 5.9/scsi-queue. Could you let me know where to find? > Sorry to not describing clearly. What I mean is that you could "add" ufshcd_vops_compl_xfer_req vop callback in your patch. Thanks, Stanley Chu ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-26 12:29 ` Stanley Chu @ 2020-06-29 7:01 ` Kiwoong Kim 2020-06-29 7:11 ` Kiwoong Kim 1 sibling, 0 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-29 7:01 UTC (permalink / raw) To: 'Stanley Chu' Cc: linux-scsi, 'Alim Akhtar', 'Avri Altman', 'James E.J. Bottomley', 'Martin K. Petersen', 'Bean Huo (beanhuo)', 'Asutosh Das', cang, bvanassche > Hi Kiwoong, > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月26日 週五 下午7:42寫道: > > > > > Hi Kiwoong, > > > > > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月20日 週六 下午3:00寫道: > > > > > > > > Some SoC specific might need command history for various reasons, > such > > > > as stacking command contexts in system memory to check for debugging > > > > in the future or scaling some DVFS knobs to boost IO throughput. > > > > > > > > What you would do with the information could be variant per SoC > > > > vendor. > > > > > > > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > > > > --- > > > > drivers/scsi/ufs/ufshcd.c | 4 ++++ > > > > drivers/scsi/ufs/ufshcd.h | 8 ++++++++ > > > > 2 files changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > > > index 52abe82..0eae3ce 100644 > > > > --- a/drivers/scsi/ufs/ufshcd.c > > > > +++ b/drivers/scsi/ufs/ufshcd.c > > > > @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct > Scsi_Host > > > *host, struct scsi_cmnd *cmd) > > > > /* issue command to the controller */ > > > > spin_lock_irqsave(hba->host->host_lock, flags); > > > > ufshcd_vops_setup_xfer_req(hba, tag, true); > > > > + if (cmd) > > > > + ufshcd_vops_cmd_log(hba, cmd, 1); > > > > ufshcd_send_command(hba, tag); > > > > out_unlock: > > > > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ > > > > -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct > > > ufs_hba *hba, > > > > /* Mark completed command as NULL in LRB */ > > > > lrbp->cmd = NULL; > > > > lrbp->compl_time_stamp = ktime_get(); > > > > + ufshcd_vops_cmd_log(hba, cmd, 2); > > > > + > > > > /* Do not touch lrbp after scsi done */ > > > > cmd->scsi_done(cmd); > > > > __ufshcd_release(hba); > > > > > > If your cmd_log vop callbacks are only existed in > "ufshcd_queuecommand" > > > and "ufshcd_transfer_req_compl", perhaps you could re-use > > > "ufshcd_vops_setup_xfer_req()" and an added "ufshcd_vops_compl_req()" > > > instead of a brand new "ufshcd_vops_cmd_log()" ? > > > > > > Thanks, > > > Stanley Chu > > > > Currently, ufshcd_vops_setup_xfer_req doesn't get scsi_cmnd variable. > > You could get scsi_cmnd by hba->lrb[tag].cmd if is_scsi_cmd is true in > your ufshcd_vops_setup_xfer_req vendor implementation. > > > Actually, when introduced this callback first, I was willing to make it > do that > > but someone gave me another idea. Then do you agree to change argument > set of the function? > > > > And I can't find ufshcd_vops_compl_req in 5.9/scsi-queue. Could you let > me know where to find? > > > > Sorry to not describing clearly. What I mean is that you could "add" > ufshcd_vops_compl_xfer_req vop callback in your patch. > > Thanks, > Stanley Chu Got it and I'll refer to what you said. Thanks. Kiwoong Kim ^ permalink raw reply [flat|nested] 18+ messages in thread
* RE: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information 2020-06-26 12:29 ` Stanley Chu 2020-06-29 7:01 ` Kiwoong Kim @ 2020-06-29 7:11 ` Kiwoong Kim 1 sibling, 0 replies; 18+ messages in thread From: Kiwoong Kim @ 2020-06-29 7:11 UTC (permalink / raw) To: linux-scsi, 'Stanley Chu' Cc: 'Alim Akhtar', 'Avri Altman', 'James E.J. Bottomley', 'Martin K. Petersen', 'Bean Huo (beanhuo)', 'Asutosh Das', cang, bvanassche > Hi Kiwoong, > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月26日 週五 下午7:42寫道: > > > > > Hi Kiwoong, > > > > > > Kiwoong Kim <kwmad.kim@samsung.com> 於 2020年6月20日 週六 下午3:00寫道: > > > > > > > > Some SoC specific might need command history for various reasons, > such > > > > as stacking command contexts in system memory to check for debugging > > > > in the future or scaling some DVFS knobs to boost IO throughput. > > > > > > > > What you would do with the information could be variant per SoC > > > > vendor. > > > > > > > > Signed-off-by: Kiwoong Kim <kwmad.kim@samsung.com> > > > > --- > > > > drivers/scsi/ufs/ufshcd.c | 4 ++++ > > > > drivers/scsi/ufs/ufshcd.h | 8 ++++++++ > > > > 2 files changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c > > > > index 52abe82..0eae3ce 100644 > > > > --- a/drivers/scsi/ufs/ufshcd.c > > > > +++ b/drivers/scsi/ufs/ufshcd.c > > > > @@ -2545,6 +2545,8 @@ static int ufshcd_queuecommand(struct > Scsi_Host > > > *host, struct scsi_cmnd *cmd) > > > > /* issue command to the controller */ > > > > spin_lock_irqsave(hba->host->host_lock, flags); > > > > ufshcd_vops_setup_xfer_req(hba, tag, true); > > > > + if (cmd) > > > > + ufshcd_vops_cmd_log(hba, cmd, 1); > > > > ufshcd_send_command(hba, tag); > > > > out_unlock: > > > > spin_unlock_irqrestore(hba->host->host_lock, flags); @@ > > > > -4890,6 +4892,8 @@ static void __ufshcd_transfer_req_compl(struct > > > ufs_hba *hba, > > > > /* Mark completed command as NULL in LRB */ > > > > lrbp->cmd = NULL; > > > > lrbp->compl_time_stamp = ktime_get(); > > > > + ufshcd_vops_cmd_log(hba, cmd, 2); > > > > + > > > > /* Do not touch lrbp after scsi done */ > > > > cmd->scsi_done(cmd); > > > > __ufshcd_release(hba); > > > > > > If your cmd_log vop callbacks are only existed in > "ufshcd_queuecommand" > > > and "ufshcd_transfer_req_compl", perhaps you could re-use > > > "ufshcd_vops_setup_xfer_req()" and an added "ufshcd_vops_compl_req()" > > > instead of a brand new "ufshcd_vops_cmd_log()" ? > > > > > > Thanks, > > > Stanley Chu > > > > Currently, ufshcd_vops_setup_xfer_req doesn't get scsi_cmnd variable. > > You could get scsi_cmnd by hba->lrb[tag].cmd if is_scsi_cmd is true in > your ufshcd_vops_setup_xfer_req vendor implementation. > > > Actually, when introduced this callback first, I was willing to make it > do that > > but someone gave me another idea. Then do you agree to change argument > set of the function? > > > > And I can't find ufshcd_vops_compl_req in 5.9/scsi-queue. Could you let > me know where to find? > > > > Sorry to not describing clearly. What I mean is that you could "add" > ufshcd_vops_compl_xfer_req vop callback in your patch. > > Thanks, > Stanley Chu Got it and I'll refer to what you said. Thanks. Kiwoong Kim ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information @ 2020-06-20 19:48 kernel test robot 0 siblings, 0 replies; 18+ messages in thread From: kernel test robot @ 2020-06-20 19:48 UTC (permalink / raw) To: kbuild [-- Attachment #1: Type: text/plain, Size: 9470 bytes --] CC: kbuild-all(a)lists.01.org In-Reply-To: <1592635992-35619-1-git-send-email-kwmad.kim@samsung.com> References: <1592635992-35619-1-git-send-email-kwmad.kim@samsung.com> TO: Kiwoong Kim <kwmad.kim@samsung.com> Hi Kiwoong, [FYI, it's a private test report for your RFC patch.] [auto build test WARNING on scsi/for-next] [also build test WARNING on mkp-scsi/for-next v5.8-rc1 next-20200618] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Kiwoong-Kim/ufs-introduce-callbacks-to-get-command-information/20200620-150310 base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next :::::: branch date: 13 hours ago :::::: commit date: 13 hours ago config: x86_64-randconfig-m001-20200620 (attached as .config) compiler: gcc-9 (Debian 9.3.0-13) 9.3.0 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> smatch warnings: drivers/scsi/ufs/ufshcd.c:2548 ufshcd_queuecommand() warn: variable dereferenced before check 'cmd' (see line 2475) # https://github.com/0day-ci/linux/commit/cff1afd5a0773e6da9106e953721996a56a9332c git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout cff1afd5a0773e6da9106e953721996a56a9332c vim +/cmd +2548 drivers/scsi/ufs/ufshcd.c 4d2b8d40dd754e Bart Van Assche 2020-01-22 2457 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2458 /** 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2459 * ufshcd_queuecommand - main entry point for SCSI requests 8aa29f192ca675 Bart Van Assche 2018-03-01 2460 * @host: SCSI host pointer 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2461 * @cmd: command from SCSI Midlayer 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2462 * 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2463 * Returns 0 for success, non-zero in case of failure 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2464 */ 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2465 static int ufshcd_queuecommand(struct Scsi_Host *host, struct scsi_cmnd *cmd) 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2466 { 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2467 struct ufshcd_lrb *lrbp; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2468 struct ufs_hba *hba; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2469 unsigned long flags; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2470 int tag; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2471 int err = 0; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2472 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2473 hba = shost_priv(host); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2474 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 @2475 tag = cmd->request->tag; 14497328b6a628 Yaniv Gardi 2016-02-01 2476 if (!ufshcd_valid_tag(hba, tag)) { 14497328b6a628 Yaniv Gardi 2016-02-01 2477 dev_err(hba->dev, 14497328b6a628 Yaniv Gardi 2016-02-01 2478 "%s: invalid command tag %d: cmd=0x%p, cmd->request=0x%p", 14497328b6a628 Yaniv Gardi 2016-02-01 2479 __func__, tag, cmd, cmd->request); 14497328b6a628 Yaniv Gardi 2016-02-01 2480 BUG(); 14497328b6a628 Yaniv Gardi 2016-02-01 2481 } 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2482 a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2483 if (!down_read_trylock(&hba->clk_scaling_lock)) a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2484 return SCSI_MLQUEUE_HOST_BUSY; a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2485 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2486 spin_lock_irqsave(hba->host->host_lock, flags); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2487 switch (hba->ufshcd_state) { 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2488 case UFSHCD_STATE_OPERATIONAL: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2489 break; 141f81651037ea Zang Leigang 2016-11-16 2490 case UFSHCD_STATE_EH_SCHEDULED: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2491 case UFSHCD_STATE_RESET: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2492 err = SCSI_MLQUEUE_HOST_BUSY; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2493 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2494 case UFSHCD_STATE_ERROR: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2495 set_host_byte(cmd, DID_ERROR); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2496 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2497 goto out_unlock; 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2498 default: 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2499 dev_WARN_ONCE(hba->dev, 1, "%s: invalid state %d\n", 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2500 __func__, hba->ufshcd_state); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2501 set_host_byte(cmd, DID_BAD_TARGET); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2502 cmd->scsi_done(cmd); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2503 goto out_unlock; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2504 } 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2505 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2506 /* if error handling is in progress, don't issue commands */ 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2507 if (ufshcd_eh_in_progress(hba)) { 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2508 set_host_byte(cmd, DID_ERROR); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2509 cmd->scsi_done(cmd); 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2510 goto out_unlock; 53c12d0ef6fcb7 Yaniv Gardi 2016-02-01 2511 } 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2512 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2513 7fabb77b3aa016 Gilad Broner 2017-02-03 2514 hba->req_abort_count = 0; 7fabb77b3aa016 Gilad Broner 2017-02-03 2515 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2516 err = ufshcd_hold(hba, true); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2517 if (err) { 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2518 err = SCSI_MLQUEUE_HOST_BUSY; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2519 goto out; 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2520 } 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2521 WARN_ON(hba->clk_gating.state != CLKS_ON); 1ab27c9cf8b63d Sahitya Tummala 2014-09-25 2522 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2523 lrbp = &hba->lrb[tag]; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2524 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2525 WARN_ON(lrbp->cmd); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2526 lrbp->cmd = cmd; 09a5a24ff36f90 Avri Altman 2018-11-22 2527 lrbp->sense_bufflen = UFS_SENSE_SIZE; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2528 lrbp->sense_buffer = cmd->sense_buffer; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2529 lrbp->task_tag = tag; 0ce147d48a3e33 Subhash Jadavani 2014-09-25 2530 lrbp->lun = ufshcd_scsi_to_upiu_lun(cmd->device->lun); b852190e589abe Yaniv Gardi 2015-05-17 2531 lrbp->intr_cmd = !ufshcd_is_intr_aggr_allowed(hba) ? true : false; e0b299e36004f5 Gilad Broner 2017-02-03 2532 lrbp->req_abort_skip = false; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2533 300bb13f5c7b1d Joao Pinto 2016-05-11 2534 ufshcd_comp_scsi_upiu(hba, lrbp); 300bb13f5c7b1d Joao Pinto 2016-05-11 2535 75b1cc4ad63afa Kiwoong Kim 2016-11-22 2536 err = ufshcd_map_sg(hba, lrbp); 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2537 if (err) { 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2538 lrbp->cmd = NULL; 17c7d35f141ef6 Can Guo 2019-12-05 2539 ufshcd_release(hba); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2540 goto out; 5a0b0cb9bee767 Sujit Reddy Thumma 2013-07-30 2541 } ad1a1b9cd67a4b Gilad Broner 2016-10-17 2542 /* Make sure descriptors are ready before ringing the doorbell */ ad1a1b9cd67a4b Gilad Broner 2016-10-17 2543 wmb(); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2544 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2545 /* issue command to the controller */ 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2546 spin_lock_irqsave(hba->host->host_lock, flags); 5905d4648e7ec2 Bart Van Assche 2020-01-22 2547 ufshcd_vops_setup_xfer_req(hba, tag, true); cff1afd5a0773e Kiwoong Kim 2020-06-20 @2548 if (cmd) cff1afd5a0773e Kiwoong Kim 2020-06-20 2549 ufshcd_vops_cmd_log(hba, cmd, 1); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2550 ufshcd_send_command(hba, tag); 3441da7ddbdedf Sujit Reddy Thumma 2014-05-26 2551 out_unlock: 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2552 spin_unlock_irqrestore(hba->host->host_lock, flags); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2553 out: a3cd5ec55f6c72 Subhash Jadavani 2017-02-03 2554 up_read(&hba->clk_scaling_lock); 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2555 return err; 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2556 } 7a3e97b0dc4bba Santosh Yaraganavi 2012-02-29 2557 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org [-- Attachment #2: config.gz --] [-- Type: application/gzip, Size: 32367 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-06-29 21:23 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CGME20200620070044epcas2p269e3c266c86c65dd0e894d8188036a30@epcas2p2.samsung.com> 2020-06-20 6:53 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Kiwoong Kim [not found] ` <CGME20200620070047epcas2p37229d52d479df9f64ee4fc14f469acf9@epcas2p3.samsung.com> 2020-06-20 6:53 ` [RFC PATCH v1 2/2] exynos-ufs: support command history Kiwoong Kim 2020-06-20 15:26 ` kernel test robot 2020-06-20 19:08 ` kernel test robot 2020-06-24 3:10 ` Bart Van Assche 2020-06-29 8:49 ` Kiwoong Kim 2020-06-29 10:08 ` Kiwoong Kim 2020-06-20 16:33 ` [RFC PATCH v1 1/2] ufs: introduce callbacks to get command information Bart Van Assche 2020-06-23 2:28 ` Kiwoong Kim 2020-06-24 2:53 ` Bart Van Assche 2020-06-22 11:35 ` Dan Carpenter 2020-06-22 11:35 ` Dan Carpenter 2020-06-25 14:05 ` Stanley Chu 2020-06-26 11:42 ` Kiwoong Kim 2020-06-26 12:29 ` Stanley Chu 2020-06-29 7:01 ` Kiwoong Kim 2020-06-29 7:11 ` Kiwoong Kim 2020-06-20 19:48 kernel test robot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.