* [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
@ 2021-10-29 7:49 Sebastian Andrzej Siewior
2021-10-29 13:41 ` John Garry
0 siblings, 1 reply; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-29 7:49 UTC (permalink / raw)
To: linux-scsi, Ketan Mukadam
Cc: James E.J. Bottomley, Martin K. Petersen, 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.
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
v1…v2:
- Properly initialize variables in be_isr_misx_th() as reported by the
bot.
- Drop jitendra.bhivare@broadcom.com and
subbu.seetharaman@broadcom.com from Cc:, the email bounces.
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 e70f69f791db6..ef09e298cca94 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,
@@ -696,6 +693,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
@@ -713,9 +752,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);
}
/**
@@ -735,6 +787,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;
@@ -774,10 +827,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)
@@ -819,9 +873,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",
@@ -847,8 +902,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",
@@ -1839,12 +1894,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;
@@ -2018,55 +2072,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)
@@ -5308,10 +5319,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 */
@@ -5344,10 +5351,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:
@@ -5379,10 +5382,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 */
@@ -5637,11 +5636,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];
@@ -5698,10 +5692,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.33.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
2021-10-29 7:49 [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler Sebastian Andrzej Siewior
@ 2021-10-29 13:41 ` John Garry
2021-10-29 13:46 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 4+ messages in thread
From: John Garry @ 2021-10-29 13:41 UTC (permalink / raw)
To: Sebastian Andrzej Siewior, linux-scsi, Ketan Mukadam
Cc: James E.J. Bottomley, Martin K. Petersen, Thomas Gleixner
> +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
> @@ -713,9 +752,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);
> }
>
> /**
> @@ -735,6 +787,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;
> @@ -774,10 +827,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)
> @@ -819,9 +873,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]);
Would it be sensible to set ONESHOT flag here? I assume that we don't
want the hard irq handler firing continuously while we poll completions
in the threaded handler. That's my understanding of how ONESHOT should
or would work... they currently seem to manually disable in
be_isr_msix() -> hwi_ring_eq_db() for the same purpose, I guess.
Thanks,
John
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
2021-10-29 13:41 ` John Garry
@ 2021-10-29 13:46 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 4+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-10-29 13:46 UTC (permalink / raw)
To: John Garry
Cc: linux-scsi, Ketan Mukadam, James E.J. Bottomley,
Martin K. Petersen, Thomas Gleixner
On 2021-10-29 14:41:53 [+0100], John Garry wrote:
> > @@ -713,9 +752,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;
> > +}
> > +
> > @@ -819,9 +873,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]);
>
> Would it be sensible to set ONESHOT flag here? I assume that we don't want
> the hard irq handler firing continuously while we poll completions in the
> threaded handler. That's my understanding of how ONESHOT should or would
> work... they currently seem to manually disable in be_isr_msix() ->
> hwi_ring_eq_db() for the same purpose, I guess.
We could. My understanding of hwi_ring_eq_db() of is that it disables
the interrupt from on HW level. This is needed _already_ in order to
continue to process the request later in softirq without the interrupt
constantly firing.
For the MSI interrupts this could be replaced "easily" and leaving the
primary handler empty. However the non-MSI interrupts may be shared and
here all handlers need the ONESHOT which may not be the case.
> Thanks,
> John
Sebastian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler.
@ 2021-10-31 20:30 kernel test robot
0 siblings, 0 replies; 4+ messages in thread
From: kernel test robot @ 2021-10-31 20:30 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 17281 bytes --]
CC: llvm(a)lists.linux.dev
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20211029074902.4fayed6mcltifgdz@linutronix.de>
References: <20211029074902.4fayed6mcltifgdz@linutronix.de>
TO: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
TO: linux-scsi(a)vger.kernel.org
TO: Ketan Mukadam <ketan.mukadam@broadcom.com>
CC: "James E.J. Bottomley" <jejb@linux.ibm.com>
CC: "Martin K. Petersen" <martin.petersen@oracle.com>
CC: Thomas Gleixner <tglx@linutronix.de>
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.15-rc7 next-20211029]
[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/20211029-155031
base: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/scsi.git for-next
:::::: branch date: 3 days ago
:::::: commit date: 3 days ago
config: i386-randconfig-c001-20211031 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project d321548c3ce987f4f21350ba1c81fdb5d4354224)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/66895bb3d61cae1257df8eb153ea1a6c9dda075a
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/20211029-155031
git checkout 66895bb3d61cae1257df8eb153ea1a6c9dda075a
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=i386 clang-analyzer
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
clang-analyzer warnings: (new ones prefixed by >>)
drivers/scsi/be2iscsi/be_main.c:3737:2: note: Loop condition is true. Entering loop body
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
^
drivers/scsi/be2iscsi/be_main.c:3738:7: note: Assuming the condition is false
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3738:3: note: Taking false branch
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^
drivers/scsi/be2iscsi/be_main.c:3737:2: note: Loop condition is true. Entering loop body
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
^
drivers/scsi/be2iscsi/be_main.c:3738:7: note: Assuming the condition is false
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3738:3: note: Taking false branch
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^
drivers/scsi/be2iscsi/be_main.c:3737:2: note: Loop condition is false. Execution continues on line 3774
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
^
drivers/scsi/be2iscsi/be_main.c:3775:6: note: 'status' is equal to 0
if (status != 0) {
^~~~~~
drivers/scsi/be2iscsi/be_main.c:3775:2: note: Taking false branch
if (status != 0) {
^
drivers/scsi/be2iscsi/be_main.c:3782:6: note: 'status' is equal to 0
if (status != 0) {
^~~~~~
drivers/scsi/be2iscsi/be_main.c:3782:2: note: Taking false branch
if (status != 0) {
^
drivers/scsi/be2iscsi/be_main.c:3787:11: note: Calling 'beiscsi_create_wrb_rings'
status = beiscsi_create_wrb_rings(phba, phwi_context, phwi_ctrlr);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3363:6: note: Assuming 'pwrb_arr' is non-null
if (!pwrb_arr) {
^~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3363:2: note: Taking false branch
if (!pwrb_arr) {
^
drivers/scsi/be2iscsi/be_main.c:3373:16: note: Assuming 'num' is < field 'cxns_per_ctrl'
for (num = 0; num < phba->params.cxns_per_ctrl; num++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3373:2: note: Loop condition is true. Entering loop body
for (num = 0; num < phba->params.cxns_per_ctrl; num++) {
^
drivers/scsi/be2iscsi/be_main.c:3374:7: note: Assuming 'num_wrb_rings' is 0
if (num_wrb_rings) {
^~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3374:3: note: Taking false branch
if (num_wrb_rings) {
^
drivers/scsi/be2iscsi/be_main.c:3373:16: note: Assuming 'num' is >= field 'cxns_per_ctrl'
for (num = 0; num < phba->params.cxns_per_ctrl; num++) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3373:2: note: Loop condition is false. Execution continues on line 3401
for (num = 0; num < phba->params.cxns_per_ctrl; num++) {
^
drivers/scsi/be2iscsi/be_main.c:3401:2: note: Loop condition is true. Entering loop body
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
^
drivers/scsi/be2iscsi/be_main.c:3402:7: note: Assuming the condition is false
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3402:3: note: Taking false branch
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^
drivers/scsi/be2iscsi/be_main.c:3401:2: note: Loop condition is true. Entering loop body
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
^
drivers/scsi/be2iscsi/be_main.c:3402:7: note: Assuming the condition is false
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3402:3: note: Taking false branch
if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
^
drivers/scsi/be2iscsi/be_main.c:3401:2: note: Loop condition is false. Execution continues on line 3409
for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
^
drivers/scsi/be2iscsi/be_main.c:3409:2: note: Loop condition is true. Entering loop body
for (i = 0; i < phba->params.cxns_per_ctrl; i++) {
^
drivers/scsi/be2iscsi/be_main.c:3410:7: note: 'ulp_count' is <= 1
if (ulp_count > 1) {
^~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:3410:3: note: Taking false branch
if (ulp_count > 1) {
^
drivers/scsi/be2iscsi/be_main.c:3421:3: note: Calling 'hwi_build_be_sgl_by_offset'
hwi_build_be_sgl_by_offset(phba, &pwrb_arr[i], &sgl);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:2981:6: note: Branch condition evaluates to a garbage value
if (sgl->va)
^~~~~~~
drivers/scsi/be2iscsi/be_main.c:4843:3: warning: Value stored to 'rc' is never read [clang-analyzer-deadcode.DeadStores]
rc = wait_event_interruptible_timeout(
^
drivers/scsi/be2iscsi/be_main.c:4843:3: note: Value stored to 'rc' is never read
>> drivers/scsi/be2iscsi/be_main.c:5392:13: warning: Array subscript is undefined [clang-analyzer-core.uninitialized.ArraySubscript]
pbe_eq = &phwi_context->be_eq[i];
^
drivers/scsi/be2iscsi/be_main.c:5416:9: note: Left side of '&&' is false
phba = container_of(work, struct beiscsi_hba, recover_port.work);
^
include/linux/kernel.h:495:61: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
drivers/scsi/be2iscsi/be_main.c:5416:9: note: Taking false branch
phba = container_of(work, struct beiscsi_hba, recover_port.work);
^
include/linux/kernel.h:495:2: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:302:3: note: expanded from macro '__compiletime_assert'
if (!(condition)) \
^
drivers/scsi/be2iscsi/be_main.c:5416:9: note: Loop condition is false. Exiting loop
phba = container_of(work, struct beiscsi_hba, recover_port.work);
^
include/linux/kernel.h:495:2: note: expanded from macro 'container_of'
BUILD_BUG_ON_MSG(!__same_type(*(ptr), ((type *)0)->member) && \
^
include/linux/build_bug.h:39:37: note: expanded from macro 'BUILD_BUG_ON_MSG'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^
include/linux/compiler_types.h:322:2: note: expanded from macro 'compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __COUNTER__)
^
include/linux/compiler_types.h:310:2: note: expanded from macro '_compiletime_assert'
__compiletime_assert(condition, msg, prefix, suffix)
^
include/linux/compiler_types.h:300:2: note: expanded from macro '__compiletime_assert'
do { \
^
drivers/scsi/be2iscsi/be_main.c:5417:2: note: Calling 'beiscsi_disable_port'
beiscsi_disable_port(phba, 0);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:5377:2: note: 'i' declared without an initial value
unsigned int i;
^~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:5379:6: note: Assuming the condition is false
if (!test_and_clear_bit(BEISCSI_HBA_ONLINE, &phba->state))
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:5379:2: note: Taking false branch
if (!test_and_clear_bit(BEISCSI_HBA_ONLINE, &phba->state))
^
drivers/scsi/be2iscsi/be_main.c:5391:7: note: 'unload' is 0
if (!unload && beiscsi_hba_in_error(phba)) {
^~~~~~
drivers/scsi/be2iscsi/be_main.c:5391:6: note: Left side of '&&' is true
if (!unload && beiscsi_hba_in_error(phba)) {
^
drivers/scsi/be2iscsi/be_main.c:5391:17: note: Assuming the condition is true
if (!unload && beiscsi_hba_in_error(phba)) {
^
drivers/scsi/be2iscsi/be_main.h:395:37: note: expanded from macro 'beiscsi_hba_in_error'
#define beiscsi_hba_in_error(phba) ((phba)->state & BEISCSI_HBA_IN_ERR)
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/scsi/be2iscsi/be_main.c:5391:2: note: Taking true branch
if (!unload && beiscsi_hba_in_error(phba)) {
^
drivers/scsi/be2iscsi/be_main.c:5392:13: note: Array subscript is undefined
pbe_eq = &phwi_context->be_eq[i];
^ ~
Suppressed 5 warnings (4 in non-user code, 1 with check filters).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.
5 warnings generated.
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:160:2: warning: Undefined or garbage value returned to caller [clang-analyzer-core.uninitialized.UndefReturn]
return ret;
^ ~~~
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:145:2: note: 'ret' declared without an initial value
int ret;
^~~~~~~
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:147:6: note: Assuming 'blob' is non-null
if (!blob)
^~~~~
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:147:2: note: Taking false branch
if (!blob)
^
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:154:7: note: Calling 'intel_guc_is_ready'
if (!intel_guc_is_ready(guc))
^~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gt/uc/intel_guc.h:232:9: note: Left side of '&&' is true
return intel_guc_is_fw_running(guc) && intel_guc_ct_enabled(&guc->ct);
^
drivers/gpu/drm/i915/gt/uc/intel_guc.h:232:2: note: Returning value, which participates in a condition later
return intel_guc_is_fw_running(guc) && intel_guc_ct_enabled(&guc->ct);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:154:7: note: Returning from 'intel_guc_is_ready'
if (!intel_guc_is_ready(guc))
vim +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(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 43387 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-31 20:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-29 7:49 [PATCH v2] scsi: be2iscsi: Replace irq_poll with threaded IRQ handler Sebastian Andrzej Siewior
2021-10-29 13:41 ` John Garry
2021-10-29 13:46 ` Sebastian Andrzej Siewior
2021-10-31 20:30 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.