* [PATCH 1/2] chelsio: cxgb: Replace the workqueue with threaded interrupt
2021-02-02 17:01 [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Sebastian Andrzej Siewior
@ 2021-02-02 17:01 ` Sebastian Andrzej Siewior
2021-02-02 17:01 ` [PATCH 2/2] chelsio: cxgb: Disable the card on error in " Sebastian Andrzej Siewior
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-02-02 17:01 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Thomas Gleixner,
Ahmed S. Darwish, Sebastian Andrzej Siewior
The external interrupt (F_PL_INTR_EXT) needs to be handled in a process
context and this is accomplished by utilizing a workqueue.
The process context can also be provided by a threaded interrupt instead
of a workqueue. The threaded interrupt can be used later for other
interrupt related processing which require non-atomic context without
using yet another workqueue. free_irq() also ensures that the thread is
done which is currently missing (the worker could continue after the
module has been removed).
Save pending flags in pending_thread_intr. Use the same mechanism
to disable F_PL_INTR_EXT as interrupt source like it is used before the
worker is scheduled. Enable the interrupt again once
t1_elmer0_ext_intr_handler() is done.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/net/ethernet/chelsio/cxgb/common.h | 5 +--
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 44 ++--------------------
drivers/net/ethernet/chelsio/cxgb/sge.c | 33 ++++++++++++++--
drivers/net/ethernet/chelsio/cxgb/sge.h | 1 +
drivers/net/ethernet/chelsio/cxgb/subr.c | 26 +++++++++----
5 files changed, 55 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/common.h b/drivers/net/ethernet/chelsio/cxgb/common.h
index 6475060649e90..e999a9b9fe6cc 100644
--- a/drivers/net/ethernet/chelsio/cxgb/common.h
+++ b/drivers/net/ethernet/chelsio/cxgb/common.h
@@ -238,7 +238,6 @@ struct adapter {
int msg_enable;
u32 mmio_len;
- struct work_struct ext_intr_handler_task;
struct adapter_params params;
/* Terminator modules. */
@@ -257,6 +256,7 @@ struct adapter {
/* guards async operations */
spinlock_t async_lock ____cacheline_aligned;
+ u32 pending_thread_intr;
u32 slow_intr_mask;
int t1powersave;
};
@@ -334,8 +334,7 @@ void t1_interrupts_enable(adapter_t *adapter);
void t1_interrupts_disable(adapter_t *adapter);
void t1_interrupts_clear(adapter_t *adapter);
int t1_elmer0_ext_intr_handler(adapter_t *adapter);
-void t1_elmer0_ext_intr(adapter_t *adapter);
-int t1_slow_intr_handler(adapter_t *adapter);
+irqreturn_t t1_slow_intr_handler(adapter_t *adapter);
int t1_link_start(struct cphy *phy, struct cmac *mac, struct link_config *lc);
const struct board_info *t1_get_board_info(unsigned int board_id);
diff --git a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
index 0e4a0f413960a..bd6f3c532d72e 100644
--- a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
+++ b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
@@ -211,9 +211,10 @@ static int cxgb_up(struct adapter *adapter)
t1_interrupts_clear(adapter);
adapter->params.has_msi = !disable_msi && !pci_enable_msi(adapter->pdev);
- err = request_irq(adapter->pdev->irq, t1_interrupt,
- adapter->params.has_msi ? 0 : IRQF_SHARED,
- adapter->name, adapter);
+ err = request_threaded_irq(adapter->pdev->irq, t1_interrupt,
+ t1_interrupt_thread,
+ adapter->params.has_msi ? 0 : IRQF_SHARED,
+ adapter->name, adapter);
if (err) {
if (adapter->params.has_msi)
pci_disable_msi(adapter->pdev);
@@ -916,41 +917,6 @@ static void mac_stats_task(struct work_struct *work)
spin_unlock(&adapter->work_lock);
}
-/*
- * Processes elmer0 external interrupts in process context.
- */
-static void ext_intr_task(struct work_struct *work)
-{
- struct adapter *adapter =
- container_of(work, struct adapter, ext_intr_handler_task);
-
- t1_elmer0_ext_intr_handler(adapter);
-
- /* Now reenable external interrupts */
- spin_lock_irq(&adapter->async_lock);
- adapter->slow_intr_mask |= F_PL_INTR_EXT;
- writel(F_PL_INTR_EXT, adapter->regs + A_PL_CAUSE);
- writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
- adapter->regs + A_PL_ENABLE);
- spin_unlock_irq(&adapter->async_lock);
-}
-
-/*
- * Interrupt-context handler for elmer0 external interrupts.
- */
-void t1_elmer0_ext_intr(struct adapter *adapter)
-{
- /*
- * Schedule a task to handle external interrupts as we require
- * a process context. We disable EXT interrupts in the interim
- * and let the task reenable them when it's done.
- */
- adapter->slow_intr_mask &= ~F_PL_INTR_EXT;
- writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
- adapter->regs + A_PL_ENABLE);
- schedule_work(&adapter->ext_intr_handler_task);
-}
-
void t1_fatal_err(struct adapter *adapter)
{
if (adapter->flags & FULL_INIT_DONE) {
@@ -1062,8 +1028,6 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
spin_lock_init(&adapter->async_lock);
spin_lock_init(&adapter->mac_lock);
- INIT_WORK(&adapter->ext_intr_handler_task,
- ext_intr_task);
INIT_DELAYED_WORK(&adapter->stats_update_task,
mac_stats_task);
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index 2d9c2b5a690a3..5aef9ae1ecfed 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -1619,11 +1619,38 @@ int t1_poll(struct napi_struct *napi, int budget)
return work_done;
}
+irqreturn_t t1_interrupt_thread(int irq, void *data)
+{
+ struct adapter *adapter = data;
+ u32 pending_thread_intr;
+
+ spin_lock_irq(&adapter->async_lock);
+ pending_thread_intr = adapter->pending_thread_intr;
+ adapter->pending_thread_intr = 0;
+ spin_unlock_irq(&adapter->async_lock);
+
+ if (!pending_thread_intr)
+ return IRQ_NONE;
+
+ if (pending_thread_intr & F_PL_INTR_EXT)
+ t1_elmer0_ext_intr_handler(adapter);
+
+ spin_lock_irq(&adapter->async_lock);
+ adapter->slow_intr_mask |= F_PL_INTR_EXT;
+
+ writel(F_PL_INTR_EXT, adapter->regs + A_PL_CAUSE);
+ writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
+ adapter->regs + A_PL_ENABLE);
+ spin_unlock_irq(&adapter->async_lock);
+
+ return IRQ_HANDLED;
+}
+
irqreturn_t t1_interrupt(int irq, void *data)
{
struct adapter *adapter = data;
struct sge *sge = adapter->sge;
- int handled;
+ irqreturn_t handled;
if (likely(responses_pending(adapter))) {
writel(F_PL_INTR_SGE_DATA, adapter->regs + A_PL_CAUSE);
@@ -1645,10 +1672,10 @@ irqreturn_t t1_interrupt(int irq, void *data)
handled = t1_slow_intr_handler(adapter);
spin_unlock(&adapter->async_lock);
- if (!handled)
+ if (handled == IRQ_NONE)
sge->stats.unhandled_irqs++;
- return IRQ_RETVAL(handled != 0);
+ return handled;
}
/*
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.h b/drivers/net/ethernet/chelsio/cxgb/sge.h
index a1ba591b34312..76516d2a8aa9e 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.h
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.h
@@ -74,6 +74,7 @@ struct sge *t1_sge_create(struct adapter *, struct sge_params *);
int t1_sge_configure(struct sge *, struct sge_params *);
int t1_sge_set_coalesce_params(struct sge *, struct sge_params *);
void t1_sge_destroy(struct sge *);
+irqreturn_t t1_interrupt_thread(int irq, void *data);
irqreturn_t t1_interrupt(int irq, void *cookie);
int t1_poll(struct napi_struct *, int);
diff --git a/drivers/net/ethernet/chelsio/cxgb/subr.c b/drivers/net/ethernet/chelsio/cxgb/subr.c
index ea0f8741d7cfd..d90ad07ff1a40 100644
--- a/drivers/net/ethernet/chelsio/cxgb/subr.c
+++ b/drivers/net/ethernet/chelsio/cxgb/subr.c
@@ -210,7 +210,7 @@ static int fpga_phy_intr_handler(adapter_t *adapter)
/*
* Slow path interrupt handler for FPGAs.
*/
-static int fpga_slow_intr(adapter_t *adapter)
+static irqreturn_t fpga_slow_intr(adapter_t *adapter)
{
u32 cause = readl(adapter->regs + A_PL_CAUSE);
@@ -238,7 +238,7 @@ static int fpga_slow_intr(adapter_t *adapter)
if (cause)
writel(cause, adapter->regs + A_PL_CAUSE);
- return cause != 0;
+ return cause == 0 ? IRQ_NONE : IRQ_HANDLED;
}
#endif
@@ -842,13 +842,14 @@ void t1_interrupts_clear(adapter_t* adapter)
/*
* Slow path interrupt handler for ASICs.
*/
-static int asic_slow_intr(adapter_t *adapter)
+static irqreturn_t asic_slow_intr(adapter_t *adapter)
{
u32 cause = readl(adapter->regs + A_PL_CAUSE);
+ irqreturn_t ret = IRQ_HANDLED;
cause &= adapter->slow_intr_mask;
if (!cause)
- return 0;
+ return IRQ_NONE;
if (cause & F_PL_INTR_SGE_ERR)
t1_sge_intr_error_handler(adapter->sge);
if (cause & F_PL_INTR_TP)
@@ -857,16 +858,25 @@ static int asic_slow_intr(adapter_t *adapter)
t1_espi_intr_handler(adapter->espi);
if (cause & F_PL_INTR_PCIX)
t1_pci_intr_handler(adapter);
- if (cause & F_PL_INTR_EXT)
- t1_elmer0_ext_intr(adapter);
+ if (cause & F_PL_INTR_EXT) {
+ /* Wake the threaded interrupt to handle external interrupts as
+ * we require a process context. We disable EXT interrupts in
+ * the interim and let the thread reenable them when it's done.
+ */
+ adapter->pending_thread_intr |= F_PL_INTR_EXT;
+ adapter->slow_intr_mask &= ~F_PL_INTR_EXT;
+ writel(adapter->slow_intr_mask | F_PL_INTR_SGE_DATA,
+ adapter->regs + A_PL_ENABLE);
+ ret = IRQ_WAKE_THREAD;
+ }
/* Clear the interrupts just processed. */
writel(cause, adapter->regs + A_PL_CAUSE);
readl(adapter->regs + A_PL_CAUSE); /* flush writes */
- return 1;
+ return ret;
}
-int t1_slow_intr_handler(adapter_t *adapter)
+irqreturn_t t1_slow_intr_handler(adapter_t *adapter)
{
#ifdef CONFIG_CHELSIO_T1_1G
if (!t1_is_asic(adapter))
--
2.30.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 2/2] chelsio: cxgb: Disable the card on error in threaded interrupt
2021-02-02 17:01 [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Sebastian Andrzej Siewior
2021-02-02 17:01 ` [PATCH 1/2] chelsio: cxgb: Replace the workqueue with threaded interrupt Sebastian Andrzej Siewior
@ 2021-02-02 17:01 ` Sebastian Andrzej Siewior
2021-02-03 1:31 ` [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Jesse Brandeburg
2021-02-04 1:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-02-02 17:01 UTC (permalink / raw)
To: netdev
Cc: David S. Miller, Jakub Kicinski, Thomas Gleixner,
Ahmed S. Darwish, Sebastian Andrzej Siewior
t1_fatal_err() is invoked from the interrupt handler. The bad part is
that it invokes (via t1_sge_stop()) del_timer_sync() and tasklet_kill().
Both functions must not be called from an interrupt because it is
possible that it will wait for the completion of the timer/tasklet it
just interrupted.
In case of a fatal error, use t1_interrupts_disable() to disable all
interrupt sources and then wake the interrupt thread with
F_PL_INTR_SGE_ERR as pending flag. The threaded-interrupt will stop the
card via t1_sge_stop() and not re-enable the interrupts again.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
drivers/net/ethernet/chelsio/cxgb/common.h | 1 -
drivers/net/ethernet/chelsio/cxgb/cxgb2.c | 10 ------
drivers/net/ethernet/chelsio/cxgb/sge.c | 20 +++++++++---
drivers/net/ethernet/chelsio/cxgb/sge.h | 2 +-
drivers/net/ethernet/chelsio/cxgb/subr.c | 38 +++++++++++++++-------
5 files changed, 44 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb/common.h b/drivers/net/ethernet/chelsio/cxgb/common.h
index e999a9b9fe6cc..0321be77366c4 100644
--- a/drivers/net/ethernet/chelsio/cxgb/common.h
+++ b/drivers/net/ethernet/chelsio/cxgb/common.h
@@ -346,7 +346,6 @@ int t1_get_board_rev(adapter_t *adapter, const struct board_info *bi,
int t1_init_hw_modules(adapter_t *adapter);
int t1_init_sw_modules(adapter_t *adapter, const struct board_info *bi);
void t1_free_sw_modules(adapter_t *adapter);
-void t1_fatal_err(adapter_t *adapter);
void t1_link_changed(adapter_t *adapter, int port_id);
void t1_link_negotiated(adapter_t *adapter, int port_id, int link_stat,
int speed, int duplex, int pause);
diff --git a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
index bd6f3c532d72e..512da98019c66 100644
--- a/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
+++ b/drivers/net/ethernet/chelsio/cxgb/cxgb2.c
@@ -917,16 +917,6 @@ static void mac_stats_task(struct work_struct *work)
spin_unlock(&adapter->work_lock);
}
-void t1_fatal_err(struct adapter *adapter)
-{
- if (adapter->flags & FULL_INIT_DONE) {
- t1_sge_stop(adapter->sge);
- t1_interrupts_disable(adapter);
- }
- pr_alert("%s: encountered fatal error, operation suspended\n",
- adapter->name);
-}
-
static const struct net_device_ops cxgb_netdev_ops = {
.ndo_open = cxgb_open,
.ndo_stop = cxgb_close,
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.c b/drivers/net/ethernet/chelsio/cxgb/sge.c
index 5aef9ae1ecfed..cda01f22c71c8 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.c
@@ -940,10 +940,11 @@ void t1_sge_intr_clear(struct sge *sge)
/*
* SGE 'Error' interrupt handler
*/
-int t1_sge_intr_error_handler(struct sge *sge)
+bool t1_sge_intr_error_handler(struct sge *sge)
{
struct adapter *adapter = sge->adapter;
u32 cause = readl(adapter->regs + A_SG_INT_CAUSE);
+ bool wake = false;
if (adapter->port[0].dev->hw_features & NETIF_F_TSO)
cause &= ~F_PACKET_TOO_BIG;
@@ -967,11 +968,14 @@ int t1_sge_intr_error_handler(struct sge *sge)
sge->stats.pkt_mismatch++;
pr_alert("%s: SGE packet mismatch\n", adapter->name);
}
- if (cause & SGE_INT_FATAL)
- t1_fatal_err(adapter);
+ if (cause & SGE_INT_FATAL) {
+ t1_interrupts_disable(adapter);
+ adapter->pending_thread_intr |= F_PL_INTR_SGE_ERR;
+ wake = true;
+ }
writel(cause, adapter->regs + A_SG_INT_CAUSE);
- return 0;
+ return wake;
}
const struct sge_intr_counts *t1_sge_get_intr_counts(const struct sge *sge)
@@ -1635,6 +1639,14 @@ irqreturn_t t1_interrupt_thread(int irq, void *data)
if (pending_thread_intr & F_PL_INTR_EXT)
t1_elmer0_ext_intr_handler(adapter);
+ /* This error is fatal, interrupts remain off */
+ if (pending_thread_intr & F_PL_INTR_SGE_ERR) {
+ pr_alert("%s: encountered fatal error, operation suspended\n",
+ adapter->name);
+ t1_sge_stop(adapter->sge);
+ return IRQ_HANDLED;
+ }
+
spin_lock_irq(&adapter->async_lock);
adapter->slow_intr_mask |= F_PL_INTR_EXT;
diff --git a/drivers/net/ethernet/chelsio/cxgb/sge.h b/drivers/net/ethernet/chelsio/cxgb/sge.h
index 76516d2a8aa9e..716705b96f265 100644
--- a/drivers/net/ethernet/chelsio/cxgb/sge.h
+++ b/drivers/net/ethernet/chelsio/cxgb/sge.h
@@ -82,7 +82,7 @@ netdev_tx_t t1_start_xmit(struct sk_buff *skb, struct net_device *dev);
void t1_vlan_mode(struct adapter *adapter, netdev_features_t features);
void t1_sge_start(struct sge *);
void t1_sge_stop(struct sge *);
-int t1_sge_intr_error_handler(struct sge *);
+bool t1_sge_intr_error_handler(struct sge *sge);
void t1_sge_intr_enable(struct sge *);
void t1_sge_intr_disable(struct sge *);
void t1_sge_intr_clear(struct sge *);
diff --git a/drivers/net/ethernet/chelsio/cxgb/subr.c b/drivers/net/ethernet/chelsio/cxgb/subr.c
index d90ad07ff1a40..310add28fcf59 100644
--- a/drivers/net/ethernet/chelsio/cxgb/subr.c
+++ b/drivers/net/ethernet/chelsio/cxgb/subr.c
@@ -170,7 +170,7 @@ void t1_link_changed(adapter_t *adapter, int port_id)
t1_link_negotiated(adapter, port_id, link_ok, speed, duplex, fc);
}
-static int t1_pci_intr_handler(adapter_t *adapter)
+static bool t1_pci_intr_handler(adapter_t *adapter)
{
u32 pcix_cause;
@@ -179,9 +179,13 @@ static int t1_pci_intr_handler(adapter_t *adapter)
if (pcix_cause) {
pci_write_config_dword(adapter->pdev, A_PCICFG_INTR_CAUSE,
pcix_cause);
- t1_fatal_err(adapter); /* PCI errors are fatal */
+ /* PCI errors are fatal */
+ t1_interrupts_disable(adapter);
+ adapter->pending_thread_intr |= F_PL_INTR_SGE_ERR;
+ pr_alert("%s: PCI error encountered.\n", adapter->name);
+ return true;
}
- return 0;
+ return false;
}
#ifdef CONFIG_CHELSIO_T1_1G
@@ -213,10 +217,13 @@ static int fpga_phy_intr_handler(adapter_t *adapter)
static irqreturn_t fpga_slow_intr(adapter_t *adapter)
{
u32 cause = readl(adapter->regs + A_PL_CAUSE);
+ irqreturn_t ret = IRQ_NONE;
cause &= ~F_PL_INTR_SGE_DATA;
- if (cause & F_PL_INTR_SGE_ERR)
- t1_sge_intr_error_handler(adapter->sge);
+ if (cause & F_PL_INTR_SGE_ERR) {
+ if (t1_sge_intr_error_handler(adapter->sge))
+ ret = IRQ_WAKE_THREAD;
+ }
if (cause & FPGA_PCIX_INTERRUPT_GMAC)
fpga_phy_intr_handler(adapter);
@@ -231,13 +238,18 @@ static irqreturn_t fpga_slow_intr(adapter_t *adapter)
/* Clear TP interrupt */
writel(tp_cause, adapter->regs + FPGA_TP_ADDR_INTERRUPT_CAUSE);
}
- if (cause & FPGA_PCIX_INTERRUPT_PCIX)
- t1_pci_intr_handler(adapter);
+ if (cause & FPGA_PCIX_INTERRUPT_PCIX) {
+ if (t1_pci_intr_handler(adapter))
+ ret = IRQ_WAKE_THREAD;
+ }
/* Clear the interrupts just processed. */
if (cause)
writel(cause, adapter->regs + A_PL_CAUSE);
+ if (ret != IRQ_NONE)
+ return ret;
+
return cause == 0 ? IRQ_NONE : IRQ_HANDLED;
}
#endif
@@ -850,14 +862,18 @@ static irqreturn_t asic_slow_intr(adapter_t *adapter)
cause &= adapter->slow_intr_mask;
if (!cause)
return IRQ_NONE;
- if (cause & F_PL_INTR_SGE_ERR)
- t1_sge_intr_error_handler(adapter->sge);
+ if (cause & F_PL_INTR_SGE_ERR) {
+ if (t1_sge_intr_error_handler(adapter->sge))
+ ret = IRQ_WAKE_THREAD;
+ }
if (cause & F_PL_INTR_TP)
t1_tp_intr_handler(adapter->tp);
if (cause & F_PL_INTR_ESPI)
t1_espi_intr_handler(adapter->espi);
- if (cause & F_PL_INTR_PCIX)
- t1_pci_intr_handler(adapter);
+ if (cause & F_PL_INTR_PCIX) {
+ if (t1_pci_intr_handler(adapter))
+ ret = IRQ_WAKE_THREAD;
+ }
if (cause & F_PL_INTR_EXT) {
/* Wake the threaded interrupt to handle external interrupts as
* we require a process context. We disable EXT interrupts in
--
2.30.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work
2021-02-02 17:01 [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Sebastian Andrzej Siewior
2021-02-02 17:01 ` [PATCH 1/2] chelsio: cxgb: Replace the workqueue with threaded interrupt Sebastian Andrzej Siewior
2021-02-02 17:01 ` [PATCH 2/2] chelsio: cxgb: Disable the card on error in " Sebastian Andrzej Siewior
@ 2021-02-03 1:31 ` Jesse Brandeburg
2021-02-03 9:03 ` Sebastian Andrzej Siewior
2021-02-04 1:50 ` patchwork-bot+netdevbpf
3 siblings, 1 reply; 6+ messages in thread
From: Jesse Brandeburg @ 2021-02-03 1:31 UTC (permalink / raw)
To: Sebastian Andrzej Siewior
Cc: netdev, David S. Miller, Jakub Kicinski, Thomas Gleixner,
Ahmed S. Darwish
Sebastian Andrzej Siewior wrote:
> Patch #2 fixes an issue in which del_timer_sync() and tasklet_kill() is
> invoked from the interrupt handler. This is probably a rare error case
> since it disables interrupts / the card in that case.
> Patch #1 converts a worker to use a threaded interrupt which is then
> also used in patch #2 instead adding another worker for this task (and
> flush_work() to synchronise vs rmmod).
>
> This has been only compile tested.
Hi! Thanks for your patch. Do all drivers that use worker threads need
to convert like this or only some?
In future revisions, please indicate the tree
you're targeting, net or net-next. ie [PATCH net-next v1] I'd also
invert the two paragraphs and talk about patch #1 first.
Jesse
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work
2021-02-03 1:31 ` [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Jesse Brandeburg
@ 2021-02-03 9:03 ` Sebastian Andrzej Siewior
0 siblings, 0 replies; 6+ messages in thread
From: Sebastian Andrzej Siewior @ 2021-02-03 9:03 UTC (permalink / raw)
To: Jesse Brandeburg
Cc: netdev, David S. Miller, Jakub Kicinski, Thomas Gleixner,
Ahmed S. Darwish
On 2021-02-02 17:31:23 [-0800], Jesse Brandeburg wrote:
> Sebastian Andrzej Siewior wrote:
>
> > Patch #2 fixes an issue in which del_timer_sync() and tasklet_kill() is
> > invoked from the interrupt handler. This is probably a rare error case
> > since it disables interrupts / the card in that case.
> > Patch #1 converts a worker to use a threaded interrupt which is then
> > also used in patch #2 instead adding another worker for this task (and
> > flush_work() to synchronise vs rmmod).
> >
> > This has been only compile tested.
>
> Hi! Thanks for your patch. Do all drivers that use worker threads need
> to convert like this or only some?
There is no need but it does makes things easier.
> In future revisions, please indicate the tree
> you're targeting, net or net-next. ie [PATCH net-next v1] I'd also
> invert the two paragraphs and talk about patch #1 first.
I left it out on purpose since I have no chance on testing the change I
made. Technically both fix something and the bug was there since day
one and it is not something you likely trigger.
> Jesse
Sebastian
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work
2021-02-02 17:01 [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Sebastian Andrzej Siewior
` (2 preceding siblings ...)
2021-02-03 1:31 ` [PATCH 0/2] chelsio: cxgb: Use threaded interrupts for deferred work Jesse Brandeburg
@ 2021-02-04 1:50 ` patchwork-bot+netdevbpf
3 siblings, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-02-04 1:50 UTC (permalink / raw)
To: Sebastian Andrzej Siewior; +Cc: netdev, davem, kuba, tglx, a.darwish
Hello:
This series was applied to netdev/net-next.git (refs/heads/master):
On Tue, 2 Feb 2021 18:01:02 +0100 you wrote:
> Patch #2 fixes an issue in which del_timer_sync() and tasklet_kill() is
> invoked from the interrupt handler. This is probably a rare error case
> since it disables interrupts / the card in that case.
> Patch #1 converts a worker to use a threaded interrupt which is then
> also used in patch #2 instead adding another worker for this task (and
> flush_work() to synchronise vs rmmod).
>
> [...]
Here is the summary with links:
- [1/2] chelsio: cxgb: Replace the workqueue with threaded interrupt
https://git.kernel.org/netdev/net-next/c/fec7fa0a750c
- [2/2] chelsio: cxgb: Disable the card on error in threaded interrupt
https://git.kernel.org/netdev/net-next/c/82154580a7f7
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 6+ messages in thread