From patchwork Thu Apr 23 18:18:01 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?UmFmYcWCIE1pxYJlY2tp?= X-Patchwork-Id: 464023 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id B2CA51400DE for ; Fri, 24 Apr 2015 04:18:10 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="verification failed; unprotected key" header.d=gmail.com header.i=@gmail.com header.b=uzj2lKk+; dkim-adsp=none (unprotected policy); dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1030484AbbDWSSF (ORCPT ); Thu, 23 Apr 2015 14:18:05 -0400 Received: from mail-ig0-f179.google.com ([209.85.213.179]:36065 "EHLO mail-ig0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1030383AbbDWSSD (ORCPT ); Thu, 23 Apr 2015 14:18:03 -0400 Received: by igblo3 with SMTP id lo3so30329694igb.1 for ; Thu, 23 Apr 2015 11:18:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:date:message-id:subject:from:to:cc:content-type; bh=1anU/lz71FSu4qoqDceKAuz/mf9cJPqKlqXj2c08Gzc=; b=uzj2lKk+1k6z1dMzoQiOROttmXPT2JnErm5HzgILU+Cw05e8vncwtwRHhZOeS5NsJ/ 2eCU/RSxwXCQFx2mbEkEtY1Vvi+KR7HX+9aRufdWPGX3B53qPDkq6G5oVPvTm8U4o7K2 yVH2dfG5ak5rAJhBd9EIoO3oWXs5sFx0jqtvy9L3oQYFo/LMnnLRVsMfxsq3m5GwKRnE mmLRFd5vZQCSqNsbLyRn0uaYXGMG+haFakAJLdm5vYp8lr1GzCMGqqK8/Z7ItF6cshOi xY++kj4V7WU3aWQqPiNzsLy+KLrmhNz4wiNIz6YAfhhqaTqSuiiLtWhX9MUxb/J565oQ OfSw== MIME-Version: 1.0 X-Received: by 10.42.204.14 with SMTP id fk14mr5158147icb.96.1429813081269; Thu, 23 Apr 2015 11:18:01 -0700 (PDT) Received: by 10.107.36.73 with HTTP; Thu, 23 Apr 2015 11:18:01 -0700 (PDT) Date: Thu, 23 Apr 2015 20:18:01 +0200 Message-ID: Subject: NAPI: Polling function requesting extra calls - difference between 3.18.11 and 4.0.0 From: =?UTF-8?B?UmFmYcWCIE1pxYJlY2tp?= To: Network Development Cc: Felix Fietkau Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Hi, Recently Felix improved bgmac driver to be smarter in situation where new packets were Tx/Rx-ed during bgmac_poll execution. It was handled in: eb64e29 bgmac: leave interrupts disabled as long as there is work to do http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=eb64e2923a886441c7b322f138b36029f3fa6a36 With above change, after handling all skb-s in bgmac_poll, bgmac checks if there are new sbk-s to be handled. If so, it doesn't call napi_complete & it doesn't enable interrupts. Above commit was merged for 4.1 kernel, but I work with OpenWrt based on older releases, so I have all bgmac changes backported to 3.8.11 and 4.0. With 3.8.11 Felix's change seems to be working fine, I did some debugging: [ 44.970000] [bgmac_interrupt] int_status & int_mask = 0x01000000 [ 44.980000] [bgmac_poll] START weight:64 [ 44.990000] [bgmac_poll] Read handled:0 skb-s [ 44.990000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:0 BGMAC_IS_RX:0 [ 45.000000] [bgmac_poll] Read skb-s < weight, calling napi_complete, en IRQs [ 45.010000] [bgmac_poll] END returning handled:0 [ 48.040000] [bgmac_interrupt] int_status & int_mask = 0x01000000 [ 48.050000] [bgmac_poll] START weight:64 [ 48.060000] [bgmac_poll] Read handled:0 skb-s [ 48.060000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:0 BGMAC_IS_RX:0 [ 48.070000] [bgmac_poll] Read skb-s < weight, calling napi_complete, en IRQs [ 48.080000] [bgmac_poll] END returning handled:0 [ 48.100000] [bgmac_poll] START weight:64 [ 48.110000] [bgmac_poll] Read handled:1 skb-s [ 48.110000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:1 BGMAC_IS_RX:1 [ 48.120000] [bgmac_poll] END more skb-s to handle, returning handled:1 [ 48.130000] [bgmac_poll] START weight:64 [ 48.130000] [bgmac_poll] Read handled:1 skb-s [ 48.140000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:0 BGMAC_IS_RX:0 [ 48.150000] [bgmac_poll] Read skb-s < weight, calling napi_complete, en IRQs [ 48.160000] [bgmac_poll] END returning handled:1 Please note that at 48.120000 bgmac decided bgmac_poll should be called again and it didn't call napi_complete & didn't enable IRQs. It simply returned 1. NAPI behaved just like we expected, bgmac_poll was called again a moment later. It handled skb-s that appeared meanwhile and ended calling napi_complete and returning 1. Now, it fails badly for me at all when using kernel 4.0.0: [ 52.800000] [bgmac_interrupt] int_status & int_mask = 0x01000000 [ 52.800000] [bgmac_poll] START weight:64 [ 52.810000] [bgmac_poll] Read handled:0 skb-s [ 52.810000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:0 BGMAC_IS_RX:0 [ 52.820000] [bgmac_poll] Read skb-s < weight, calling napi_complete, en IRQs [ 52.830000] [bgmac_poll] END returning handled:0 [ 54.610000] [bgmac_interrupt] int_status & int_mask = 0x01000000 [ 54.620000] [bgmac_poll] START weight:64 [ 54.630000] [bgmac_poll] Read handled:0 skb-s [ 54.630000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:0 BGMAC_IS_RX:0 [ 54.640000] [bgmac_poll] Read skb-s < weight, calling napi_complete, en IRQs [ 54.650000] [bgmac_poll] END returning handled:0 [ 55.900000] [bgmac_interrupt] int_status & int_mask = 0x01000000 [ 55.910000] [bgmac_poll] START weight:64 [ 55.920000] [bgmac_poll] Read handled:1 skb-s [ 55.920000] [bgmac_poll] Read IRQ status again BGMAC_IS_TX0:1 BGMAC_IS_RX:1 [ 55.930000] [bgmac_poll] END more skb-s to handle, returning handled:1 As you can see, at 55.930000 bgmac also decided bgmac_poll should be called again. It didn't call napi_complete, didn't enable IRQs. It simply returned 1. Just like with kernel 3.8.11. It this case however, NAPI never called bgmac_poll again. It resulted in bgmac hanging (IRQs are off, bgmac expects bgmac_poll to be called). Can you help us with this, please? Does bgmac use NAPI incorrectly? Were there some important changes in 3.19 or 4.0? diff --git a/drivers/net/ethernet/broadcom/bgmac.c b/drivers/net/ethernet/broadcom/bgmac.c index 4929932..40ce34d 100644 --- a/drivers/net/ethernet/broadcom/bgmac.c +++ b/drivers/net/ethernet/broadcom/bgmac.c @@ -1219,6 +1219,7 @@ static irqreturn_t bgmac_interrupt(int irq, void *dev_id) struct bgmac *bgmac = netdev_priv(dev_id); u32 int_status = bgmac_read(bgmac, BGMAC_INT_STATUS); + pr_info("[%s] int_status:0x%08x & int_mask:0x%08x = 0x%08x\n", __FUNCTION__, int_status, bgmac->int_mask, int_status & bgmac->int_mask); int_status &= bgmac->int_mask; if (!int_status) @@ -1240,22 +1241,31 @@ static int bgmac_poll(struct napi_struct *napi, int weight) { struct bgmac *bgmac = container_of(napi, struct bgmac, napi); int handled = 0; + u32 int_status; + pr_info("[%s] START weight:%d\n", __FUNCTION__, weight); /* Ack */ bgmac_write(bgmac, BGMAC_INT_STATUS, ~0); bgmac_dma_tx_free(bgmac, &bgmac->tx_ring[0]); handled += bgmac_dma_rx_read(bgmac, &bgmac->rx_ring[0], weight); + pr_info("[%s] Read handled:%d skb-s\n", __FUNCTION__, handled); /* Poll again if more events arrived in the meantime */ - if (bgmac_read(bgmac, BGMAC_INT_STATUS) & (BGMAC_IS_TX0 | BGMAC_IS_RX)) + int_status = bgmac_read(bgmac, BGMAC_INT_STATUS); + pr_info("[%s] Read IRQ status again BGMAC_IS_TX0:%d BGMAC_IS_RX:%d\n", __FUNCTION__, !!(int_status & BGMAC_IS_TX0), !!(int_status & BGMAC_IS_RX)); + if (int_status & (BGMAC_IS_TX0 | BGMAC_IS_RX)) { + pr_info("[%s] END more skb-s to handle, returning handled:%d\n", __FUNCTION__, handled); return handled; + } if (handled < weight) { + pr_info("[%s] Read skb-s < weight, calling napi_complete, en IRQs\n", __FUNCTION__); napi_complete(napi); bgmac_chip_intrs_on(bgmac); } + pr_info("[%s] END returning handled:%d\n", __FUNCTION__, handled); return handled; }