All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Gleixner <tglx@linutronix.de>
To: linux-usb@vger.kernel.org, netdev@vger.kernel.org
Cc: Michal Svec <msvec@suse.com>,
	"David S. Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	Hayes Wang <hayeswang@realtek.com>,
	Thierry Reding <treding@nvidia.com>,
	Lee Jones <lee.jones@linaro.org>, Borislav Petkov <bp@alien8.de>
Subject: [PATCH RFC] r8152: Ensure that napi_schedule() is handled
Date: Fri, 14 May 2021 12:17:19 +0200	[thread overview]
Message-ID: <877dk162mo.ffs@nanos.tec.linutronix.de> (raw)

From: Thomas Gleixner <tglx@linutronix.de>
Date: Fri, 14 May 2021 11:46:08 +0200

The driver invokes napi_schedule() in several places from task
context. napi_schedule() raises the NET_RX softirq bit and relies on the
calling context to ensure that the softirq is handled. That's usually on
return from interrupt or on the outermost local_bh_enable().

But that's not the case here which causes the soft interrupt handling to be
delayed to the next interrupt or local_bh_enable(). If the task in which
context this is invoked is the last runnable task on a CPU and the CPU goes
idle before an interrupt arrives or a local_bh_disable/enable() pair
handles the pending soft interrupt then the NOHZ idle code emits the
following warning.

  NOHZ tick-stop error: Non-RCU local softirq work is pending, handler #08!!!

Prevent this by wrapping the napi_schedule() invocation from task context
into a local_bh_disable/enable() pair.

Reported-by: Michal Svec <msvec@suse.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---

Note: That's not the first incident of this. Shouldn't napi_schedule()
      have a debug check (under lockdep) to catch this?

---
 drivers/net/usb/r8152.c |   19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

--- a/drivers/net/usb/r8152.c
+++ b/drivers/net/usb/r8152.c
@@ -1543,6 +1543,17 @@ void write_mii_word(struct net_device *n
 	r8152_mdio_write(tp, reg, val);
 }
 
+/*
+ * Wrapper around napi_schedule() to ensure that the raised network softirq
+ * is actually handled.
+ */
+static void r8152_napi_schedule(struct napi_struct *napi)
+{
+	local_bh_disable();
+	napi_schedule(napi);
+	local_bh_enable();
+}
+
 static int
 r8152_submit_rx(struct r8152 *tp, struct rx_agg *agg, gfp_t mem_flags);
 
@@ -1753,7 +1764,7 @@ static void read_bulk_callback(struct ur
 		spin_lock_irqsave(&tp->rx_lock, flags);
 		list_add_tail(&agg->list, &tp->rx_done);
 		spin_unlock_irqrestore(&tp->rx_lock, flags);
-		napi_schedule(&tp->napi);
+		r8152_napi_schedule(&tp->napi);
 		return;
 	case -ESHUTDOWN:
 		rtl_set_unplug(tp);
@@ -2640,7 +2651,7 @@ int r8152_submit_rx(struct r8152 *tp, st
 		netif_err(tp, rx_err, tp->netdev,
 			  "Couldn't submit rx[%p], ret = %d\n", agg, ret);
 
-		napi_schedule(&tp->napi);
+		r8152_napi_schedule(&tp->napi);
 	}
 
 	return ret;
@@ -8202,7 +8213,7 @@ static int rtl8152_post_reset(struct usb
 	usb_submit_urb(tp->intr_urb, GFP_KERNEL);
 
 	if (!list_empty(&tp->rx_done))
-		napi_schedule(&tp->napi);
+		r8152_napi_schedule(&tp->napi);
 
 	return 0;
 }
@@ -8256,7 +8267,7 @@ static int rtl8152_runtime_resume(struct
 		smp_mb__after_atomic();
 
 		if (!list_empty(&tp->rx_done))
-			napi_schedule(&tp->napi);
+			r8152_napi_schedule(&tp->napi);
 
 		usb_submit_urb(tp->intr_urb, GFP_NOIO);
 	} else {

             reply	other threads:[~2021-05-14 10:17 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-14 10:17 Thomas Gleixner [this message]
2021-05-14 19:38 ` [PATCH RFC] r8152: Ensure that napi_schedule() is handled Jakub Kicinski
2021-05-14 20:25   ` Thomas Gleixner
2021-05-14 20:46     ` Jakub Kicinski
2021-05-14 21:10       ` Thomas Gleixner
2021-05-14 21:41         ` Jakub Kicinski
2021-05-14 23:23           ` Thomas Gleixner
2021-05-14 23:36             ` Jakub Kicinski
2021-05-15 13:09             ` Peter Zijlstra
2021-05-15 19:06               ` Thomas Gleixner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877dk162mo.ffs@nanos.tec.linutronix.de \
    --to=tglx@linutronix.de \
    --cc=bp@alien8.de \
    --cc=davem@davemloft.net \
    --cc=hayeswang@realtek.com \
    --cc=kuba@kernel.org \
    --cc=lee.jones@linaro.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=msvec@suse.com \
    --cc=netdev@vger.kernel.org \
    --cc=treding@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.