From: Benjamin Poirier <bpoirier@suse.com>
Date: Wed, 24 Apr 2019 16:06:47 +0900
Subject: qlge: Remove irq_cnt
Patch-mainline: Not yet, submitted v1, will submit v2 after v5.4-rc1
References: bsc#1106061
qlge uses an irq enable/disable refcounting scheme that is:
* poorly implemented
Uses a spin_lock to protect accesses to the irq_cnt atomic
variable.
* buggy
Breaks when there is not a 1:1 sequence of irq - napi_poll, such as
when using SO_BUSY_POLL.
* unnecessary
The purpose or irq_cnt is to reduce irq control writes when
multiple work items result from one irq: the irq is re-enabled
after all work is done.
Analysis of the irq handler shows that there is only one case where
there might be two workers scheduled at once, and those have
separate irq masking bits.
Therefore, remove irq_cnt.
Additionally, we get a performance improvement:
perf stat -e cycles -a -r5 super_netperf 100 -H 192.168.33.1 -t TCP_RR
Before:
628560
628056
622103
622744
627202
[...]
268,803,947,669 cycles ( +- 0.09% )
After:
636300
634106
634984
638555
634188
[...]
259,237,291,449 cycles ( +- 0.19% )
Signed-off-by: Benjamin Poirier <bpoirier@suse.com>
---
drivers/net/ethernet/qlogic/qlge/qlge.h | 7 -
drivers/net/ethernet/qlogic/qlge/qlge_main.c | 98 +++++++--------------------
drivers/net/ethernet/qlogic/qlge/qlge_mpi.c | 1
3 files changed, 27 insertions(+), 79 deletions(-)
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -1982,11 +1982,6 @@ struct intr_context {
u32 intr_dis_mask; /* value/mask used to disable this intr */
u32 intr_read_mask; /* value/mask used to read this intr */
char name[IFNAMSIZ * 2];
- atomic_t irq_cnt; /* irq_cnt is used in single vector
- * environment. It's incremented for each
- * irq handler that is scheduled. When each
- * handler finishes it decrements irq_cnt and
- * enables interrupts if it's zero. */
irq_handler_t handler;
};
@@ -2074,7 +2069,6 @@ struct ql_adapter {
u32 port; /* Port number this adapter */
spinlock_t adapter_lock;
- spinlock_t hw_lock;
spinlock_t stats_lock;
/* PCI Bus Relative Register Addresses */
@@ -2220,7 +2214,6 @@ void ql_mpi_reset_work(struct work_struc
void ql_mpi_core_to_log(struct work_struct *work);
int ql_wait_reg_rdy(struct ql_adapter *qdev, u32 reg, u32 bit, u32 ebit);
void ql_queue_asic_error(struct ql_adapter *qdev);
-u32 ql_enable_completion_interrupt(struct ql_adapter *qdev, u32 intr);
void ql_set_ethtool_ops(struct net_device *ndev);
int ql_read_xgmac_reg64(struct ql_adapter *qdev, u32 reg, u64 *data);
void ql_mpi_idc_work(struct work_struct *work);
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -625,75 +625,26 @@ static void ql_disable_interrupts(struct
ql_write32(qdev, INTR_EN, (INTR_EN_EI << 16));
}
-/* If we're running with multiple MSI-X vectors then we enable on the fly.
- * Otherwise, we may have multiple outstanding workers and don't want to
- * enable until the last one finishes. In this case, the irq_cnt gets
- * incremented every time we queue a worker and decremented every time
- * a worker finishes. Once it hits zero we enable the interrupt.
- */
-u32 ql_enable_completion_interrupt(struct ql_adapter *qdev, u32 intr)
+static void ql_enable_completion_interrupt(struct ql_adapter *qdev, u32 intr)
{
- u32 var = 0;
- unsigned long hw_flags = 0;
- struct intr_context *ctx = qdev->intr_context + intr;
+ struct intr_context *ctx = &qdev->intr_context[intr];
- if (likely(test_bit(QL_MSIX_ENABLED, &qdev->flags) && intr)) {
- /* Always enable if we're MSIX multi interrupts and
- * it's not the default (zeroeth) interrupt.
- */
- ql_write32(qdev, INTR_EN,
- ctx->intr_en_mask);
- var = ql_read32(qdev, STS);
- return var;
- }
-
- spin_lock_irqsave(&qdev->hw_lock, hw_flags);
- if (atomic_dec_and_test(&ctx->irq_cnt)) {
- ql_write32(qdev, INTR_EN,
- ctx->intr_en_mask);
- var = ql_read32(qdev, STS);
- }
- spin_unlock_irqrestore(&qdev->hw_lock, hw_flags);
- return var;
+ ql_write32(qdev, INTR_EN, ctx->intr_en_mask);
}
-static u32 ql_disable_completion_interrupt(struct ql_adapter *qdev, u32 intr)
+static void ql_disable_completion_interrupt(struct ql_adapter *qdev, u32 intr)
{
- u32 var = 0;
- struct intr_context *ctx;
+ struct intr_context *ctx = &qdev->intr_context[intr];
- /* HW disables for us if we're MSIX multi interrupts and
- * it's not the default (zeroeth) interrupt.
- */
- if (likely(test_bit(QL_MSIX_ENABLED, &qdev->flags) && intr))
- return 0;
-
- ctx = qdev->intr_context + intr;
- spin_lock(&qdev->hw_lock);
- if (!atomic_read(&ctx->irq_cnt)) {
- ql_write32(qdev, INTR_EN,
- ctx->intr_dis_mask);
- var = ql_read32(qdev, STS);
- }
- atomic_inc(&ctx->irq_cnt);
- spin_unlock(&qdev->hw_lock);
- return var;
+ ql_write32(qdev, INTR_EN, ctx->intr_dis_mask);
}
static void ql_enable_all_completion_interrupts(struct ql_adapter *qdev)
{
int i;
- for (i = 0; i < qdev->intr_count; i++) {
- /* The enable call does a atomic_dec_and_test
- * and enables only if the result is zero.
- * So we precharge it here.
- */
- if (unlikely(!test_bit(QL_MSIX_ENABLED, &qdev->flags) ||
- i == 0))
- atomic_set(&qdev->intr_context[i].irq_cnt, 1);
- ql_enable_completion_interrupt(qdev, i);
- }
+ for (i = 0; i < qdev->intr_count; i++)
+ ql_enable_completion_interrupt(qdev, i);
}
static int ql_validate_flash(struct ql_adapter *qdev, u32 size, const char *str)
@@ -2500,21 +2451,22 @@ static irqreturn_t qlge_isr(int irq, voi
u32 var;
int work_done = 0;
- spin_lock(&qdev->hw_lock);
- if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
- netif_printk(qdev, intr, KERN_DEBUG, qdev->ndev,
- "Shared Interrupt, Not ours!\n");
- spin_unlock(&qdev->hw_lock);
- return IRQ_NONE;
- }
- spin_unlock(&qdev->hw_lock);
+ /* Experience shows that when using INTx interrupts, interrupts must
+ * be masked manually.
+ * When using MSI mode, INTR_EN_EN must be explicitly disabled
+ * (even though it is auto-masked), otherwise a later command to
+ * enable it is not effective.
+ */
+ if (!test_bit(QL_MSIX_ENABLED, &qdev->flags))
+ ql_disable_completion_interrupt(qdev, 0);
- var = ql_disable_completion_interrupt(qdev, intr_context->intr);
+ var = ql_read32(qdev, STS);
/*
* Check for fatal error.
*/
if (var & STS_FE) {
+ ql_disable_completion_interrupt(qdev, 0);
ql_queue_asic_error(qdev);
netdev_err(qdev->ndev, "Got fatal error, STS = %x.\n", var);
var = ql_read32(qdev, ERR_STS);
@@ -2534,7 +2486,6 @@ static irqreturn_t qlge_isr(int irq, voi
*/
netif_err(qdev, intr, qdev->ndev,
"Got MPI processor interrupt.\n");
- ql_disable_completion_interrupt(qdev, intr_context->intr);
ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16));
queue_delayed_work_on(smp_processor_id(),
qdev->workqueue, &qdev->mpi_work, 0);
@@ -2550,11 +2501,18 @@ static irqreturn_t qlge_isr(int irq, voi
if (var & intr_context->irq_mask) {
netif_info(qdev, intr, qdev->ndev,
"Waking handler for rx_ring[0].\n");
- ql_disable_completion_interrupt(qdev, intr_context->intr);
napi_schedule(&rx_ring->napi);
work_done++;
+ } else {
+ /* Experience shows that the device sometimes signals an
+ * interrupt but no work is scheduled from this function.
+ * Nevertheless, the interrupt is auto-masked. Therefore, we
+ * systematically re-enable the interrupt if we didn't
+ * schedule napi.
+ */
+ ql_enable_completion_interrupt(qdev, 0);
}
- ql_enable_completion_interrupt(qdev, intr_context->intr);
+
return work_done ? IRQ_HANDLED : IRQ_NONE;
}
@@ -3567,7 +3525,6 @@ static int ql_request_irq(struct ql_adap
ql_resolve_queues_to_irqs(qdev);
for (i = 0; i < qdev->intr_count; i++, intr_context++) {
- atomic_set(&intr_context->irq_cnt, 0);
if (test_bit(QL_MSIX_ENABLED, &qdev->flags)) {
status = request_irq(qdev->msi_x_entry[i].vector,
intr_context->handler,
@@ -4652,7 +4609,6 @@ static int ql_init_device(struct pci_dev
goto err_out2;
}
qdev->msg_enable = netif_msg_init(debug, default_msg);
- spin_lock_init(&qdev->hw_lock);
spin_lock_init(&qdev->stats_lock);
if (qlge_mpi_coredump) {
--- a/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_mpi.c
@@ -1256,7 +1256,6 @@ void ql_mpi_work(struct work_struct *wor
/* End polled mode for MPI */
ql_write32(qdev, INTR_MASK, (INTR_MASK_PI << 16) | INTR_MASK_PI);
mutex_unlock(&qdev->mpi_mutex);
- ql_enable_completion_interrupt(qdev, 0);
}
void ql_mpi_reset_work(struct work_struct *work)