On 9/14/20 10:54 PM, Jakub Kicinski wrote:
> On Sat, 12 Sep 2020 21:36:29 +0200 Hauke Mehrtens wrote:
>> The napi_schedule() call will only schedule the NAPI if it is not
>> already running. To make sure that we do not deactivate interrupts
>> without scheduling NAPI only deactivate the interrupts in case NAPI also
>> gets scheduled.
>>
>> Signed-off-by: Hauke Mehrtens <ha...@hauke-m.de>
>> ---
>> drivers/net/ethernet/lantiq_xrx200.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/lantiq_xrx200.c
>> b/drivers/net/ethernet/lantiq_xrx200.c
>> index abee7d61074c..635ff3a5dcfb 100644
>> --- a/drivers/net/ethernet/lantiq_xrx200.c
>> +++ b/drivers/net/ethernet/lantiq_xrx200.c
>> @@ -345,10 +345,12 @@ static irqreturn_t xrx200_dma_irq(int irq, void *ptr)
>> {
>> struct xrx200_chan *ch = ptr;
>>
>> - ltq_dma_disable_irq(&ch->dma);
>> - ltq_dma_ack_irq(&ch->dma);
>> + if (napi_schedule_prep(&ch->napi)) {
>> + __napi_schedule(&ch->napi);
>> + ltq_dma_disable_irq(&ch->dma);
>> + }
>>
>> - napi_schedule(&ch->napi);
>> + ltq_dma_ack_irq(&ch->dma);
>>
>> return IRQ_HANDLED;
>> }
>
> The patches look good to me, but I wonder why you don't want to always
> disable the IRQ here? You're guaranteed that NAPI will get called, or it
> was disabled. In both cases seems like disabling the IRQ can only help.
> This was more or less copied from the mtk_handle_irq_rx() function of this driver: drivers/net/ethernet/mediatek/mtk_eth_soc.c My assumption was that napi_schedule_prep() could return false and then NAPI would not be triggered and the IRQ would be deactivated. In such a case the network would hang. I observed such hangs of the TX, which were mostly fixed by the first patch in this series, but I still saw some of them even with that first patch. With this last patch I did not see them any more, but I am not 100% if all possible cases are eliminated. Hauke
signature.asc
Description: OpenPGP digital signature
