| Message ID | 20191126222013.1904785-2-bigeasy@linutronix.de |
|---|---|
| State | Changes Requested |
| Delegated to: | David Miller |
| Headers | show |
| Series | __napi_schedule_irqoff() used in wrong context | expand |
On 11/26/19 4:20 PM, Sebastian Andrzej Siewior wrote: > The driver uses __napi_schedule_irqoff() which is fine as long as it is > invoked with disabled interrupts by everybody. Since the commit > mentioned below the driver may invoke xgbe_isr_task() in tasklet/softirq > context. This may lead to list corruption if another driver uses > __napi_schedule_irqoff() in IRQ context. > > Use __napi_schedule() which safe to use from IRQ and softirq context. > > Fixes: 85b85c853401d ("amd-xgbe: Re-issue interrupt if interrupt status not cleared") > Cc: Tom Lendacky <thomas.lendacky@amd.com> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > index 98f8f20331544..3bd20f7651207 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > @@ -514,7 +514,7 @@ static void xgbe_isr_task(unsigned long data) > xgbe_disable_rx_tx_ints(pdata); > > /* Turn on polling */ > - __napi_schedule_irqoff(&pdata->napi); > + __napi_schedule(&pdata->napi); > } > } else { > /* Don't clear Rx/Tx status if doing per channel DMA >
On 2019-12-02 11:19:00 [-0600], Tom Lendacky wrote: > On 11/26/19 4:20 PM, Sebastian Andrzej Siewior wrote: > > The driver uses __napi_schedule_irqoff() which is fine as long as it is > > invoked with disabled interrupts by everybody. Since the commit > > mentioned below the driver may invoke xgbe_isr_task() in tasklet/softirq > > context. This may lead to list corruption if another driver uses > > __napi_schedule_irqoff() in IRQ context. > > > > Use __napi_schedule() which safe to use from IRQ and softirq context. > > > > Fixes: 85b85c853401d ("amd-xgbe: Re-issue interrupt if interrupt status not cleared") > > Cc: Tom Lendacky <thomas.lendacky@amd.com> > > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > > Acked-by: Tom Lendacky <thomas.lendacky@amd.com> *ping* This still applies and is independent of the conversation in 2/2. > > --- > > drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > > index 98f8f20331544..3bd20f7651207 100644 > > --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c > > @@ -514,7 +514,7 @@ static void xgbe_isr_task(unsigned long data) > > xgbe_disable_rx_tx_ints(pdata); > > > > /* Turn on polling */ > > - __napi_schedule_irqoff(&pdata->napi); > > + __napi_schedule(&pdata->napi); > > } > > } else { > > /* Don't clear Rx/Tx status if doing per channel DMA > > Sebastian
On 4/16/20 6:52 AM, Sebastian Andrzej Siewior wrote: > On 2019-12-02 11:19:00 [-0600], Tom Lendacky wrote: >> On 11/26/19 4:20 PM, Sebastian Andrzej Siewior wrote: >>> The driver uses __napi_schedule_irqoff() which is fine as long as it is >>> invoked with disabled interrupts by everybody. Since the commit >>> mentioned below the driver may invoke xgbe_isr_task() in tasklet/softirq >>> context. This may lead to list corruption if another driver uses >>> __napi_schedule_irqoff() in IRQ context. >>> >>> Use __napi_schedule() which safe to use from IRQ and softirq context. >>> >>> Fixes: 85b85c853401d ("amd-xgbe: Re-issue interrupt if interrupt status not cleared") >>> Cc: Tom Lendacky <thomas.lendacky@amd.com> >>> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> >> >> Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > > *ping* > This still applies and is independent of the conversation in 2/2. Then resend this patch alone, including the Acked-by you collected so far. Thanks. > >>> --- >>> drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c >>> index 98f8f20331544..3bd20f7651207 100644 >>> --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c >>> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c >>> @@ -514,7 +514,7 @@ static void xgbe_isr_task(unsigned long data) >>> xgbe_disable_rx_tx_ints(pdata); >>> >>> /* Turn on polling */ >>> - __napi_schedule_irqoff(&pdata->napi); >>> + __napi_schedule(&pdata->napi); >>> } >>> } else { >>> /* Don't clear Rx/Tx status if doing per channel DMA >>> > > Sebastian >
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c index 98f8f20331544..3bd20f7651207 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c @@ -514,7 +514,7 @@ static void xgbe_isr_task(unsigned long data) xgbe_disable_rx_tx_ints(pdata); /* Turn on polling */ - __napi_schedule_irqoff(&pdata->napi); + __napi_schedule(&pdata->napi); } } else { /* Don't clear Rx/Tx status if doing per channel DMA
The driver uses __napi_schedule_irqoff() which is fine as long as it is invoked with disabled interrupts by everybody. Since the commit mentioned below the driver may invoke xgbe_isr_task() in tasklet/softirq context. This may lead to list corruption if another driver uses __napi_schedule_irqoff() in IRQ context. Use __napi_schedule() which safe to use from IRQ and softirq context. Fixes: 85b85c853401d ("amd-xgbe: Re-issue interrupt if interrupt status not cleared") Cc: Tom Lendacky <thomas.lendacky@amd.com> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> --- drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)