* [PATCH v2 REPOST] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
@ 2021-12-02 20:05 Sebastian Andrzej Siewior
2021-12-03 22:58 ` kernel test robot
0 siblings, 1 reply; 2+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-12-02 20:05 UTC (permalink / raw)
To: linux-scsi
Cc: Ketan Mukadam, James E.J. Bottomley, Martin K. Petersen,
Subbu Seetharaman, Jitendra Bhivare, Thomas Gleixner
The driver is using irq_poll() (a NAPI like generic interface) for
completing individual I/O requests.
This could be replaced with threaded interrupts. The threaded interrupt
runs as a RT thread with priority 50-FIFO. It should run as the first
task after the interrupt unless a task with higher priority is active.
It can be interrupt by another hardware interrupt or a softirq.
This has been compile tested only. I'm interested if it causes any
regressions, improves the situation or neither.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
REPOST of
https://https://lkml.kernel.org/r/.kernel.org/all/20211029074902.4fayed6mcltifgdz@linutronix.de/
v1…v2:
- Properly initialize variables in be_isr_misx_th() as reported by the
bot.
drivers/scsi/be2iscsi/Kconfig | 1 -
drivers/scsi/be2iscsi/be.h | 3 +-
drivers/scsi/be2iscsi/be_iscsi.c | 8 +-
drivers/scsi/be2iscsi/be_main.c | 146 ++++++++++++++-----------------
drivers/scsi/be2iscsi/be_main.h | 2 +-
5 files changed, 75 insertions(+), 85 deletions(-)
diff --git a/drivers/scsi/be2iscsi/Kconfig b/drivers/scsi/be2iscsi/Kconfig
index 958c9b46ec787..0118b2a765caf 100644
--- a/drivers/scsi/be2iscsi/Kconfig
+++ b/drivers/scsi/be2iscsi/Kconfig
@@ -4,7 +4,6 @@ config BE2ISCSI
depends on PCI && SCSI && NET
select SCSI_ISCSI_ATTRS
select ISCSI_BOOT_SYSFS
- select IRQ_POLL
help
This driver implements the iSCSI functionality for Emulex
diff --git a/drivers/scsi/be2iscsi/be.h b/drivers/scsi/be2iscsi/be.h
index 4c58a02590c7b..8623886810310 100644
--- a/drivers/scsi/be2iscsi/be.h
+++ b/drivers/scsi/be2iscsi/be.h
@@ -12,7 +12,7 @@
#include <linux/pci.h>
#include <linux/if_vlan.h>
-#include <linux/irq_poll.h>
+
#define FW_VER_LEN 32
#define MCC_Q_LEN 128
#define MCC_CQ_LEN 256
@@ -90,7 +90,6 @@ struct be_eq_obj {
struct beiscsi_hba *phba;
struct be_queue_info *cq;
struct work_struct mcc_work; /* Work Item */
- struct irq_poll iopoll;
};
struct be_mcc_obj {
diff --git a/drivers/scsi/be2iscsi/be_iscsi.c b/drivers/scsi/be2iscsi/be_iscsi.c
index 8aeaddc93b167..e15d5c7ef611e 100644
--- a/drivers/scsi/be2iscsi/be_iscsi.c
+++ b/drivers/scsi/be2iscsi/be_iscsi.c
@@ -1224,15 +1224,17 @@ static void beiscsi_flush_cq(struct beiscsi_hba *phba)
struct be_eq_obj *pbe_eq;
struct hwi_controller *phwi_ctrlr;
struct hwi_context_memory *phwi_context;
+ struct pci_dev *pcidev;
phwi_ctrlr = phba->phwi_ctrlr;
phwi_context = phwi_ctrlr->phwi_ctxt;
+ pcidev = phba->pcidev;
for (i = 0; i < phba->num_cpus; i++) {
pbe_eq = &phwi_context->be_eq[i];
- irq_poll_disable(&pbe_eq->iopoll);
- beiscsi_process_cq(pbe_eq, BE2_MAX_NUM_CQ_PROC);
- irq_poll_enable(&pbe_eq->iopoll);
+ disable_irq(pci_irq_vector(pcidev, i));
+ beiscsi_process_cq(pbe_eq);
+ enable_irq(pci_irq_vector(pcidev, i));
}
}
diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
index ab55681145f8a..4dc4b840a17dc 100644
--- a/drivers/scsi/be2iscsi/be_main.c
+++ b/drivers/scsi/be2iscsi/be_main.c
@@ -35,7 +35,6 @@
#include <linux/iscsi_boot_sysfs.h>
#include <linux/module.h>
#include <linux/bsg-lib.h>
-#include <linux/irq_poll.h>
#include <scsi/libiscsi.h>
#include <scsi/scsi_bsg_iscsi.h>
@@ -51,7 +50,6 @@
#include "be_mgmt.h"
#include "be_cmds.h"
-static unsigned int be_iopoll_budget = 10;
static unsigned int be_max_phys_size = 64;
static unsigned int enable_msix = 1;
@@ -59,7 +57,6 @@ MODULE_DESCRIPTION(DRV_DESC " " BUILD_STR);
MODULE_VERSION(BUILD_STR);
MODULE_AUTHOR("Emulex Corporation");
MODULE_LICENSE("GPL");
-module_param(be_iopoll_budget, int, 0);
module_param(enable_msix, int, 0);
module_param(be_max_phys_size, uint, S_IRUGO);
MODULE_PARM_DESC(be_max_phys_size,
@@ -699,6 +696,48 @@ static irqreturn_t be_isr_mcc(int irq, void *dev_id)
return IRQ_HANDLED;
}
+static irqreturn_t be_iopoll(struct be_eq_obj *pbe_eq)
+{
+ struct beiscsi_hba *phba;
+ unsigned int ret, io_events;
+ struct be_eq_entry *eqe = NULL;
+ struct be_queue_info *eq;
+
+ phba = pbe_eq->phba;
+ if (beiscsi_hba_in_error(phba))
+ return IRQ_NONE;
+
+ io_events = 0;
+ eq = &pbe_eq->q;
+ eqe = queue_tail_node(eq);
+ while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32] &
+ EQE_VALID_MASK) {
+ AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
+ queue_tail_inc(eq);
+ eqe = queue_tail_node(eq);
+ io_events++;
+ }
+ hwi_ring_eq_db(phba, eq->id, 1, io_events, 0, 1);
+
+ ret = beiscsi_process_cq(pbe_eq);
+ pbe_eq->cq_count += ret;
+ beiscsi_log(phba, KERN_INFO,
+ BEISCSI_LOG_CONFIG | BEISCSI_LOG_IO,
+ "BM_%d : rearm pbe_eq->q.id =%d ret %d\n",
+ pbe_eq->q.id, ret);
+ if (!beiscsi_hba_in_error(phba))
+ hwi_ring_eq_db(phba, pbe_eq->q.id, 0, 0, 1, 1);
+
+ return IRQ_HANDLED;
+}
+
+static irqreturn_t be_isr_misx_th(int irq, void *dev_id)
+{
+ struct be_eq_obj *pbe_eq = dev_id;
+
+ return be_iopoll(pbe_eq);
+}
+
/**
* be_isr_msix - The isr routine of the driver.
* @irq: Not used
@@ -716,9 +755,22 @@ static irqreturn_t be_isr_msix(int irq, void *dev_id)
phba = pbe_eq->phba;
/* disable interrupt till iopoll completes */
hwi_ring_eq_db(phba, eq->id, 1, 0, 0, 1);
- irq_poll_sched(&pbe_eq->iopoll);
- return IRQ_HANDLED;
+ return IRQ_WAKE_THREAD;
+}
+
+static irqreturn_t be_isr_thread(int irq, void *dev_id)
+{
+ struct beiscsi_hba *phba;
+ struct hwi_controller *phwi_ctrlr;
+ struct hwi_context_memory *phwi_context;
+ struct be_eq_obj *pbe_eq;
+
+ phba = dev_id;
+ phwi_ctrlr = phba->phwi_ctrlr;
+ phwi_context = phwi_ctrlr->phwi_ctxt;
+ pbe_eq = &phwi_context->be_eq[0];
+ return be_iopoll(pbe_eq);
}
/**
@@ -738,6 +790,7 @@ static irqreturn_t be_isr(int irq, void *dev_id)
struct be_ctrl_info *ctrl;
struct be_eq_obj *pbe_eq;
int isr, rearm;
+ irqreturn_t ret;
phba = dev_id;
ctrl = &phba->ctrl;
@@ -777,10 +830,11 @@ static irqreturn_t be_isr(int irq, void *dev_id)
/* rearm for MCCQ */
rearm = 1;
}
+ ret = IRQ_HANDLED;
if (io_events)
- irq_poll_sched(&pbe_eq->iopoll);
+ ret = IRQ_WAKE_THREAD;
hwi_ring_eq_db(phba, eq->id, 0, (io_events + mcc_events), rearm, 1);
- return IRQ_HANDLED;
+ return ret;
}
static void beiscsi_free_irqs(struct beiscsi_hba *phba)
@@ -822,9 +876,10 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
goto free_msix_irqs;
}
- ret = request_irq(pci_irq_vector(pcidev, i),
- be_isr_msix, 0, phba->msi_name[i],
- &phwi_context->be_eq[i]);
+ ret = request_threaded_irq(pci_irq_vector(pcidev, i),
+ be_isr_msix, be_isr_misx_th,
+ 0, phba->msi_name[i],
+ &phwi_context->be_eq[i]);
if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
"BM_%d : %s-Failed to register msix for i = %d\n",
@@ -850,8 +905,8 @@ static int beiscsi_init_irqs(struct beiscsi_hba *phba)
}
} else {
- ret = request_irq(pcidev->irq, be_isr, IRQF_SHARED,
- "beiscsi", phba);
+ ret = request_threaded_irq(pcidev->irq, be_isr, be_isr_thread,
+ IRQF_SHARED, "beiscsi", phba);
if (ret) {
beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
"BM_%d : %s-Failed to register irq\n",
@@ -1842,12 +1897,11 @@ static void beiscsi_mcc_work(struct work_struct *work)
/**
* beiscsi_process_cq()- Process the Completion Queue
* @pbe_eq: Event Q on which the Completion has come
- * @budget: Max number of events to processed
*
* return
* Number of Completion Entries processed.
**/
-unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq, int budget)
+unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq)
{
struct be_queue_info *cq;
struct sol_cqe *sol;
@@ -2021,55 +2075,12 @@ unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq, int budget)
queue_tail_inc(cq);
sol = queue_tail_node(cq);
num_processed++;
- if (total == budget)
- break;
}
hwi_ring_cq_db(phba, cq->id, num_processed, 1);
return total;
}
-static int be_iopoll(struct irq_poll *iop, int budget)
-{
- unsigned int ret, io_events;
- struct beiscsi_hba *phba;
- struct be_eq_obj *pbe_eq;
- struct be_eq_entry *eqe = NULL;
- struct be_queue_info *eq;
-
- pbe_eq = container_of(iop, struct be_eq_obj, iopoll);
- phba = pbe_eq->phba;
- if (beiscsi_hba_in_error(phba)) {
- irq_poll_complete(iop);
- return 0;
- }
-
- io_events = 0;
- eq = &pbe_eq->q;
- eqe = queue_tail_node(eq);
- while (eqe->dw[offsetof(struct amap_eq_entry, valid) / 32] &
- EQE_VALID_MASK) {
- AMAP_SET_BITS(struct amap_eq_entry, valid, eqe, 0);
- queue_tail_inc(eq);
- eqe = queue_tail_node(eq);
- io_events++;
- }
- hwi_ring_eq_db(phba, eq->id, 1, io_events, 0, 1);
-
- ret = beiscsi_process_cq(pbe_eq, budget);
- pbe_eq->cq_count += ret;
- if (ret < budget) {
- irq_poll_complete(iop);
- beiscsi_log(phba, KERN_INFO,
- BEISCSI_LOG_CONFIG | BEISCSI_LOG_IO,
- "BM_%d : rearm pbe_eq->q.id =%d ret %d\n",
- pbe_eq->q.id, ret);
- if (!beiscsi_hba_in_error(phba))
- hwi_ring_eq_db(phba, pbe_eq->q.id, 0, 0, 1, 1);
- }
- return ret;
-}
-
static void
hwi_write_sgl_v2(struct iscsi_wrb *pwrb, struct scatterlist *sg,
unsigned int num_sg, struct beiscsi_io_task *io_task)
@@ -5311,10 +5322,6 @@ static int beiscsi_enable_port(struct beiscsi_hba *phba)
phwi_ctrlr = phba->phwi_ctrlr;
phwi_context = phwi_ctrlr->phwi_ctxt;
- for (i = 0; i < phba->num_cpus; i++) {
- pbe_eq = &phwi_context->be_eq[i];
- irq_poll_init(&pbe_eq->iopoll, be_iopoll_budget, be_iopoll);
- }
i = (phba->pcidev->msix_enabled) ? i : 0;
/* Work item for MCC handling */
@@ -5347,10 +5354,6 @@ static int beiscsi_enable_port(struct beiscsi_hba *phba)
return 0;
cleanup_port:
- for (i = 0; i < phba->num_cpus; i++) {
- pbe_eq = &phwi_context->be_eq[i];
- irq_poll_disable(&pbe_eq->iopoll);
- }
hwi_cleanup_port(phba);
disable_msix:
@@ -5382,10 +5385,6 @@ static void beiscsi_disable_port(struct beiscsi_hba *phba, int unload)
beiscsi_free_irqs(phba);
pci_free_irq_vectors(phba->pcidev);
- for (i = 0; i < phba->num_cpus; i++) {
- pbe_eq = &phwi_context->be_eq[i];
- irq_poll_disable(&pbe_eq->iopoll);
- }
cancel_delayed_work_sync(&phba->eqd_update);
cancel_work_sync(&phba->boot_work);
/* WQ might be running cancel queued mcc_work if we are not exiting */
@@ -5640,11 +5639,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
phwi_ctrlr = phba->phwi_ctrlr;
phwi_context = phwi_ctrlr->phwi_ctxt;
- for (i = 0; i < phba->num_cpus; i++) {
- pbe_eq = &phwi_context->be_eq[i];
- irq_poll_init(&pbe_eq->iopoll, be_iopoll_budget, be_iopoll);
- }
-
i = (phba->pcidev->msix_enabled) ? i : 0;
/* Work item for MCC handling */
pbe_eq = &phwi_context->be_eq[i];
@@ -5701,10 +5695,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
hwi_disable_intr(phba);
beiscsi_free_irqs(phba);
disable_iopoll:
- for (i = 0; i < phba->num_cpus; i++) {
- pbe_eq = &phwi_context->be_eq[i];
- irq_poll_disable(&pbe_eq->iopoll);
- }
destroy_workqueue(phba->wq);
free_twq:
hwi_cleanup_port(phba);
diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
index 98977c0700f1a..5c0e54eaf9e99 100644
--- a/drivers/scsi/be2iscsi/be_main.h
+++ b/drivers/scsi/be2iscsi/be_main.h
@@ -799,7 +799,7 @@ void hwi_ring_cq_db(struct beiscsi_hba *phba,
unsigned int id, unsigned int num_processed,
unsigned char rearm);
-unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq, int budget);
+unsigned int beiscsi_process_cq(struct be_eq_obj *pbe_eq);
void beiscsi_process_mcc_cq(struct beiscsi_hba *phba);
struct pdu_nop_out {
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2 REPOST] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
2021-12-02 20:05 [PATCH v2 REPOST] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler Sebastian Andrzej Siewior
@ 2021-12-03 22:58 ` kernel test robot
0 siblings, 0 replies; 2+ messages in thread
From: kernel test robot @ 2021-12-03 22:58 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, linux-scsi
Cc: llvm, kbuild-all, Ketan Mukadam, James E.J. Bottomley,
Martin K. Petersen, Subbu Seetharaman, Jitendra Bhivare,
Thomas Gleixner
Hi Sebastian,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on jejb-scsi/for-next]
[also build test WARNING on mkp-scsi/for-next v5.16-rc3 next-20211203]
[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]
url: https://github.com/0day-ci/linux/commits/Sebastian-Andrzej-Siewior/scsi-be2iscsi-Replace-irq_poll-with-threaded-IRQ-handler/20211203-040851
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
config: riscv-buildonly-randconfig-r002-20211203 (https://download.01.org/0day-ci/archive/20211204/202112040653.cszvYjJj-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d30fcadf07ee552f20156ea90be2fdb54cb9cb08)
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
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/46ae6b75188a7c555a80135814182b82a0fad7c8
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Sebastian-Andrzej-Siewior/scsi-be2iscsi-Replace-irq_poll-with-threaded-IRQ-handler/20211203-040851
git checkout 46ae6b75188a7c555a80135814182b82a0fad7c8
# save the config file to linux build tree
mkdir build_dir
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash drivers/ata/ drivers/infiniband/sw/rxe/ drivers/scsi/be2iscsi/
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All warnings (new ones prefixed by >>):
>> drivers/scsi/be2iscsi/be_main.c:5392:33: warning: variable 'i' is uninitialized when used here [-Wuninitialized]
pbe_eq = &phwi_context->be_eq[i];
^
drivers/scsi/be2iscsi/be_main.c:5377:16: note: initialize the variable 'i' to silence this warning
unsigned int i;
^
= 0
1 warning generated.
vim +/i +5392 drivers/scsi/be2iscsi/be_main.c
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5363
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5364 /*
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5365 * beiscsi_disable_port()- Disable port and cleanup driver resources.
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5366 * This is called in HBA error handling and driver removal.
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5367 * @phba: Instance Priv structure
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5368 * @unload: indicate driver is unloading
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5369 *
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5370 * Free the OS and HW resources held by the driver
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5371 **/
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5372 static void beiscsi_disable_port(struct beiscsi_hba *phba, int unload)
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5373 {
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5374 struct hwi_context_memory *phwi_context;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5375 struct hwi_controller *phwi_ctrlr;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5376 struct be_eq_obj *pbe_eq;
831488669a334e Christoph Hellwig 2017-01-13 5377 unsigned int i;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5378
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5379 if (!test_and_clear_bit(BEISCSI_HBA_ONLINE, &phba->state))
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5380 return;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5381
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5382 phwi_ctrlr = phba->phwi_ctrlr;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5383 phwi_context = phwi_ctrlr->phwi_ctxt;
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5384 hwi_disable_intr(phba);
45371aa398c647 Jitendra Bhivare 2017-10-10 5385 beiscsi_free_irqs(phba);
831488669a334e Christoph Hellwig 2017-01-13 5386 pci_free_irq_vectors(phba->pcidev);
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5387
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5388 cancel_delayed_work_sync(&phba->eqd_update);
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5389 cancel_work_sync(&phba->boot_work);
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5390 /* WQ might be running cancel queued mcc_work if we are not exiting */
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5391 if (!unload && beiscsi_hba_in_error(phba)) {
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 @5392 pbe_eq = &phwi_context->be_eq[i];
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5393 cancel_work_sync(&pbe_eq->mcc_work);
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5394 }
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5395 hwi_cleanup_port(phba);
dd940972f36779 Jitendra Bhivare 2016-12-13 5396 beiscsi_cleanup_port(phba);
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5397 }
d1d5ca887c0ee6 Jitendra Bhivare 2016-08-19 5398
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-12-03 22:58 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-02 20:05 [PATCH v2 REPOST] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler Sebastian Andrzej Siewior
2021-12-03 22:58 ` kernel test robot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).