| Message ID | 20241008065950.23431-1-ed.tsai@mediatek.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: ufs: ufs-mediatek: configure individual LU queue flags | expand |
On 10/7/24 11:59 PM, ed.tsai@mediatek.com wrote: > +static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev) > +{ > + struct ufs_hba *hba = shost_priv(sdev->host); > + > + dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun); > + if (sdev->lun == 2) > + blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue); > +} There are no block drivers in the upstream kernel that set QUEUE_FLAG_SAME_FORCE. An explanation is missing from the patch description why this flag is set from the UFS driver instead of by writing the value "2" into /sys/class/block/$bdev/queue/rq_affinity. Additionally, an explanation is missing why QUEUE_FLAG_SAME_FORCE is set but QUEUE_FLAG_SAME_COMP not. Bart.
On Tue, 2024-10-08 at 10:38 -0700, Bart Van Assche wrote: > On 10/7/24 11:59 PM, ed.tsai@mediatek.com wrote: > > +static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev) > > +{ > > +struct ufs_hba *hba = shost_priv(sdev->host); > > + > > +dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun); > > +if (sdev->lun == 2) > > +blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue); > > +} > > There are no block drivers in the upstream kernel that set > QUEUE_FLAG_SAME_FORCE. An explanation is missing from the patch > description why this flag is set from the UFS driver instead of by > writing the value "2" into /sys/class/block/$bdev/queue/rq_affinity. The LU probing is completed asynchronously by another thread, which means that "sdc" cannot be guaranteed to be LU2. We do not need to change the queue settings at runtime. Therefore, a simpler and more intuitive approach is to set its flag once the SCSI device is confirmed to be ready. > Additionally, an explanation is missing why QUEUE_FLAG_SAME_FORCE is > set but QUEUE_FLAG_SAME_COMP not. QUEUE_FLAG_SAME_COMP is the default flag for blk mq queue. As this stage, it should be set by default. > > Bart.
Hi, kernel test robot noticed the following build errors: [auto build test ERROR on mkp-scsi/for-next] [also build test ERROR on jejb-scsi/for-next linus/master v6.12-rc2 next-20241008] [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/ed-tsai-mediatek-com/scsi-ufs-ufs-mediatek-configure-individual-LU-queue-flags/20241008-153700 base: https://git.kernel.org/pub/scm/linux/kernel/git/mkp/scsi.git for-next patch link: https://lore.kernel.org/r/20241008065950.23431-1-ed.tsai%40mediatek.com patch subject: [PATCH] scsi: ufs: ufs-mediatek: configure individual LU queue flags config: arm-allyesconfig (https://download.01.org/0day-ci/archive/20241009/202410091257.SE04cckD-lkp@intel.com/config) compiler: arm-linux-gnueabi-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241009/202410091257.SE04cckD-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202410091257.SE04cckD-lkp@intel.com/ All errors (new ones prefixed by >>): drivers/ufs/host/ufs-mediatek.c: In function 'ufs_mtk_config_scsi_dev': >> drivers/ufs/host/ufs-mediatek.c:1789:65: error: 'struct scsi_device' has no member named 'reqeust_queue'; did you mean 'request_queue'? 1789 | blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue); | ^~~~~~~~~~~~~ | request_queue Kconfig warnings: (for reference only) WARNING: unmet direct dependencies detected for GET_FREE_REGION Depends on [n]: SPARSEMEM [=n] Selected by [y]: - RESOURCE_KUNIT_TEST [=y] && RUNTIME_TESTING_MENU [=y] && KUNIT [=y] vim +1789 drivers/ufs/host/ufs-mediatek.c 1782 1783 static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev) 1784 { 1785 struct ufs_hba *hba = shost_priv(sdev->host); 1786 1787 dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun); 1788 if (sdev->lun == 2) > 1789 blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue); 1790 } 1791
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c index 7cab103112e1..be50b86269bf 100644 --- a/drivers/ufs/core/ufshcd.c +++ b/drivers/ufs/core/ufshcd.c @@ -5253,6 +5253,9 @@ static int ufshcd_device_configure(struct scsi_device *sdev, */ sdev->silence_suspend = 1; + if (hba->vops && hba->vops->config_scsi_dev) + hba->vops->config_scsi_dev(sdev); + ufshcd_crypto_register(hba, q); return 0; diff --git a/drivers/ufs/host/ufs-mediatek.c b/drivers/ufs/host/ufs-mediatek.c index 9a5919434c4e..0b57623edca5 100644 --- a/drivers/ufs/host/ufs-mediatek.c +++ b/drivers/ufs/host/ufs-mediatek.c @@ -1780,6 +1780,15 @@ static int ufs_mtk_config_esi(struct ufs_hba *hba) return ufs_mtk_config_mcq(hba, true); } +static void ufs_mtk_config_scsi_dev(struct scsi_device *sdev) +{ + struct ufs_hba *hba = shost_priv(sdev->host); + + dev_dbg(hba->dev, "lu %llu scsi device configured", sdev->lun); + if (sdev->lun == 2) + blk_queue_flag_set(QUEUE_FLAG_SAME_FORCE, sdev->reqeust_queue); +} + /* * struct ufs_hba_mtk_vops - UFS MTK specific variant operations * @@ -1809,6 +1818,7 @@ static const struct ufs_hba_variant_ops ufs_hba_mtk_vops = { .op_runtime_config = ufs_mtk_op_runtime_config, .mcq_config_resource = ufs_mtk_mcq_config_resource, .config_esi = ufs_mtk_config_esi, + .config_scsi_dev = ufs_mtk_config_scsi_dev, }; /** diff --git a/include/ufs/ufshcd.h b/include/ufs/ufshcd.h index a95282b9f743..800d79dc91fc 100644 --- a/include/ufs/ufshcd.h +++ b/include/ufs/ufshcd.h @@ -383,6 +383,7 @@ struct ufs_hba_variant_ops { int (*get_outstanding_cqs)(struct ufs_hba *hba, unsigned long *ocqs); int (*config_esi)(struct ufs_hba *hba); + void (*config_scsi_dev)(struct scsi_device *sdev); }; /* clock gating state */