From: Eric Dumazet <eric.dumazet@gmail.com>
To: Oliver Neukum <oneukum@suse.com>,
Marek Szyprowski <m.szyprowski@samsung.com>
Cc: 'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>,
Linux USB Mailing List <linux-usb@vger.kernel.org>,
netdev@vger.kernel.org, Dean Jenkins <Dean_Jenkins@mentor.com>
Subject: [net] net: usbnet: fix potential deadlock on 32bit hosts
Date: Mon, 05 Mar 2018 11:41:13 -0800 [thread overview]
Message-ID: <1520278873.109662.14.camel@gmail.com> (raw)
From: Eric Dumazet <edumazet@google.com>
Marek reported a LOCKDEP issue occurring on 32bit host,
that we tracked down to the fact that usbnet could either
run from soft or hard irqs.
This patch adds u64_stats_update_begin_irqsave() and
u64_stats_update_end_irqrestore() helpers to solve this case.
[ 17.768040] ================================
[ 17.772239] WARNING: inconsistent lock state
[ 17.776511] 4.16.0-rc3-next-20180227-00007-g876c53a7493c #453 Not tainted
[ 17.783329] --------------------------------
[ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
asix_rx_fixup_internal+0x188/0x288
[ 17.806790] {IN-HARDIRQ-W} state was registered at:
[ 17.811677] tx_complete+0x100/0x208
[ 17.815319] __usb_hcd_giveback_urb+0x60/0xf0
[ 17.819770] xhci_giveback_urb_in_irq+0xa8/0x240
[ 17.824469] xhci_td_cleanup+0xf4/0x16c
[ 17.828367] xhci_irq+0xe74/0x2240
[ 17.831827] usb_hcd_irq+0x24/0x38
[ 17.835343] __handle_irq_event_percpu+0x98/0x510
[ 17.840111] handle_irq_event_percpu+0x1c/0x58
[ 17.844623] handle_irq_event+0x38/0x5c
[ 17.848519] handle_fasteoi_irq+0xa4/0x138
[ 17.852681] generic_handle_irq+0x18/0x28
[ 17.856760] __handle_domain_irq+0x6c/0xe4
[ 17.860941] gic_handle_irq+0x54/0xa0
[ 17.864666] __irq_svc+0x70/0xb0
[ 17.867964] arch_cpu_idle+0x20/0x3c
[ 17.871578] arch_cpu_idle+0x20/0x3c
[ 17.875190] do_idle+0x144/0x218
[ 17.878468] cpu_startup_entry+0x18/0x1c
[ 17.882454] start_kernel+0x394/0x400
[ 17.886177] irq event stamp: 161912
[ 17.889616] hardirqs last enabled at (161912): [<7bedfacf>]
__netdev_alloc_skb+0xcc/0x140
[ 17.897893] hardirqs last disabled at (161911): [<d58261d0>]
__netdev_alloc_skb+0x94/0x140
[ 17.904903] exynos5-hsi2c 12ca0000.i2c: tx timeout
[ 17.906116] softirqs last enabled at (161904): [<387102ff>]
irq_enter+0x78/0x80
[ 17.906123] softirqs last disabled at (161905): [<cf4c628e>]
irq_exit+0x134/0x158
[ 17.925722].
[ 17.925722] other info that might help us debug this:
[ 17.933435] Possible unsafe locking scenario:
[ 17.933435].
[ 17.940331] CPU0
[ 17.942488] ----
[ 17.944894] lock(&syncp->seq#5);
[ 17.948274] <Interrupt>
[ 17.950847] lock(&syncp->seq#5);
[ 17.954386].
[ 17.954386] *** DEADLOCK ***
[ 17.954386].
[ 17.962422] no locks held by swapper/0/0.
Fixes: c8b5d129ee29 ("net: usbnet: support 64bit stats")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Greg Ungerer <gerg@linux-m68k.org>
---
drivers/net/usb/usbnet.c | 10 ++++++----
include/linux/u64_stats_sync.h | 22 ++++++++++++++++++++++
2 files changed, 28 insertions(+), 4 deletions(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8a22ff67b0268a588428c61c6a6211e3c6c2a02a..d9eea8cfe6cb9a3bf8d0d4ce9198af9bccf9c757 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -315,6 +315,7 @@ static void __usbnet_status_stop_force(struct usbnet *dev)
void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
{
struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->stats64);
+ unsigned long flags;
int status;
if (test_bit(EVENT_RX_PAUSED, &dev->flags)) {
@@ -326,10 +327,10 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
if (skb->protocol == 0)
skb->protocol = eth_type_trans (skb, dev->net);
- u64_stats_update_begin(&stats64->syncp);
+ flags = u64_stats_update_begin_irqsave(&stats64->syncp);
stats64->rx_packets++;
stats64->rx_bytes += skb->len;
- u64_stats_update_end(&stats64->syncp);
+ u64_stats_update_end_irqrestore(&stats64->syncp, flags);
netif_dbg(dev, rx_status, dev->net, "< rx, len %zu, type 0x%x\n",
skb->len + sizeof (struct ethhdr), skb->protocol);
@@ -1248,11 +1249,12 @@ static void tx_complete (struct urb *urb)
if (urb->status == 0) {
struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->stats64);
+ unsigned long flags;
- u64_stats_update_begin(&stats64->syncp);
+ flags = u64_stats_update_begin_irqsave(&stats64->syncp);
stats64->tx_packets += entry->packets;
stats64->tx_bytes += entry->length;
- u64_stats_update_end(&stats64->syncp);
+ u64_stats_update_end_irqrestore(&stats64->syncp, flags);
} else {
dev->net->stats.tx_errors++;
diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
index 5bdbd9f49395f883ca2dc5aa0d7bbde11f379063..07ee0f84a46caa9e2b1c446f96009f63b3b99f50 100644
--- a/include/linux/u64_stats_sync.h
+++ b/include/linux/u64_stats_sync.h
@@ -90,6 +90,28 @@ static inline void u64_stats_update_end(struct u64_stats_sync *syncp)
#endif
}
+static inline unsigned long
+u64_stats_update_begin_irqsave(struct u64_stats_sync *syncp)
+{
+ unsigned long flags = 0;
+
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+ local_irq_save(flags);
+ write_seqcount_begin(&syncp->seq);
+#endif
+ return flags;
+}
+
+static inline void
+u64_stats_update_end_irqrestore(struct u64_stats_sync *syncp,
+ unsigned long flags)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+ write_seqcount_end(&syncp->seq);
+ local_irq_restore(flags);
+#endif
+}
+
static inline void u64_stats_update_begin_raw(struct u64_stats_sync *syncp)
{
#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
WARNING: multiple messages have this Message-ID (diff)
From: Eric Dumazet <eric.dumazet@gmail.com>
To: Oliver Neukum <oneukum@suse.com>,
Marek Szyprowski <m.szyprowski@samsung.com>
Cc: 'Linux Samsung SOC' <linux-samsung-soc@vger.kernel.org>,
Linux USB Mailing List <linux-usb@vger.kernel.org>,
netdev@vger.kernel.org, Dean Jenkins <Dean_Jenkins@mentor.com>
Subject: [PATCH net] net: usbnet: fix potential deadlock on 32bit hosts
Date: Mon, 05 Mar 2018 11:41:13 -0800 [thread overview]
Message-ID: <1520278873.109662.14.camel@gmail.com> (raw)
In-Reply-To: <1520276942.109662.9.camel@gmail.com>
From: Eric Dumazet <edumazet@google.com>
Marek reported a LOCKDEP issue occurring on 32bit host,
that we tracked down to the fact that usbnet could either
run from soft or hard irqs.
This patch adds u64_stats_update_begin_irqsave() and
u64_stats_update_end_irqrestore() helpers to solve this case.
[ 17.768040] ================================
[ 17.772239] WARNING: inconsistent lock state
[ 17.776511] 4.16.0-rc3-next-20180227-00007-g876c53a7493c #453 Not tainted
[ 17.783329] --------------------------------
[ 17.787580] inconsistent {IN-HARDIRQ-W} -> {HARDIRQ-ON-W} usage.
[ 17.793607] swapper/0/0 [HC0[0]:SC1[1]:HE1:SE0] takes:
[ 17.798751] (&syncp->seq#5){?.-.}, at: [<9b22e5f0>]
asix_rx_fixup_internal+0x188/0x288
[ 17.806790] {IN-HARDIRQ-W} state was registered at:
[ 17.811677] tx_complete+0x100/0x208
[ 17.815319] __usb_hcd_giveback_urb+0x60/0xf0
[ 17.819770] xhci_giveback_urb_in_irq+0xa8/0x240
[ 17.824469] xhci_td_cleanup+0xf4/0x16c
[ 17.828367] xhci_irq+0xe74/0x2240
[ 17.831827] usb_hcd_irq+0x24/0x38
[ 17.835343] __handle_irq_event_percpu+0x98/0x510
[ 17.840111] handle_irq_event_percpu+0x1c/0x58
[ 17.844623] handle_irq_event+0x38/0x5c
[ 17.848519] handle_fasteoi_irq+0xa4/0x138
[ 17.852681] generic_handle_irq+0x18/0x28
[ 17.856760] __handle_domain_irq+0x6c/0xe4
[ 17.860941] gic_handle_irq+0x54/0xa0
[ 17.864666] __irq_svc+0x70/0xb0
[ 17.867964] arch_cpu_idle+0x20/0x3c
[ 17.871578] arch_cpu_idle+0x20/0x3c
[ 17.875190] do_idle+0x144/0x218
[ 17.878468] cpu_startup_entry+0x18/0x1c
[ 17.882454] start_kernel+0x394/0x400
[ 17.886177] irq event stamp: 161912
[ 17.889616] hardirqs last enabled at (161912): [<7bedfacf>]
__netdev_alloc_skb+0xcc/0x140
[ 17.897893] hardirqs last disabled at (161911): [<d58261d0>]
__netdev_alloc_skb+0x94/0x140
[ 17.904903] exynos5-hsi2c 12ca0000.i2c: tx timeout
[ 17.906116] softirqs last enabled at (161904): [<387102ff>]
irq_enter+0x78/0x80
[ 17.906123] softirqs last disabled at (161905): [<cf4c628e>]
irq_exit+0x134/0x158
[ 17.925722].
[ 17.925722] other info that might help us debug this:
[ 17.933435] Possible unsafe locking scenario:
[ 17.933435].
[ 17.940331] CPU0
[ 17.942488] ----
[ 17.944894] lock(&syncp->seq#5);
[ 17.948274] <Interrupt>
[ 17.950847] lock(&syncp->seq#5);
[ 17.954386].
[ 17.954386] *** DEADLOCK ***
[ 17.954386].
[ 17.962422] no locks held by swapper/0/0.
Fixes: c8b5d129ee29 ("net: usbnet: support 64bit stats")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: Greg Ungerer <gerg@linux-m68k.org>
---
drivers/net/usb/usbnet.c | 10 ++++++----
include/linux/u64_stats_sync.h | 22 ++++++++++++++++++++++
2 files changed, 28 insertions(+), 4 deletions(-)
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 8a22ff67b0268a588428c61c6a6211e3c6c2a02a..d9eea8cfe6cb9a3bf8d0d4ce9198af9bccf9c757 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -315,6 +315,7 @@ static void __usbnet_status_stop_force(struct usbnet *dev)
void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
{
struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->stats64);
+ unsigned long flags;
int status;
if (test_bit(EVENT_RX_PAUSED, &dev->flags)) {
@@ -326,10 +327,10 @@ void usbnet_skb_return (struct usbnet *dev, struct sk_buff *skb)
if (skb->protocol == 0)
skb->protocol = eth_type_trans (skb, dev->net);
- u64_stats_update_begin(&stats64->syncp);
+ flags = u64_stats_update_begin_irqsave(&stats64->syncp);
stats64->rx_packets++;
stats64->rx_bytes += skb->len;
- u64_stats_update_end(&stats64->syncp);
+ u64_stats_update_end_irqrestore(&stats64->syncp, flags);
netif_dbg(dev, rx_status, dev->net, "< rx, len %zu, type 0x%x\n",
skb->len + sizeof (struct ethhdr), skb->protocol);
@@ -1248,11 +1249,12 @@ static void tx_complete (struct urb *urb)
if (urb->status == 0) {
struct pcpu_sw_netstats *stats64 = this_cpu_ptr(dev->stats64);
+ unsigned long flags;
- u64_stats_update_begin(&stats64->syncp);
+ flags = u64_stats_update_begin_irqsave(&stats64->syncp);
stats64->tx_packets += entry->packets;
stats64->tx_bytes += entry->length;
- u64_stats_update_end(&stats64->syncp);
+ u64_stats_update_end_irqrestore(&stats64->syncp, flags);
} else {
dev->net->stats.tx_errors++;
diff --git a/include/linux/u64_stats_sync.h b/include/linux/u64_stats_sync.h
index 5bdbd9f49395f883ca2dc5aa0d7bbde11f379063..07ee0f84a46caa9e2b1c446f96009f63b3b99f50 100644
--- a/include/linux/u64_stats_sync.h
+++ b/include/linux/u64_stats_sync.h
@@ -90,6 +90,28 @@ static inline void u64_stats_update_end(struct u64_stats_sync *syncp)
#endif
}
+static inline unsigned long
+u64_stats_update_begin_irqsave(struct u64_stats_sync *syncp)
+{
+ unsigned long flags = 0;
+
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+ local_irq_save(flags);
+ write_seqcount_begin(&syncp->seq);
+#endif
+ return flags;
+}
+
+static inline void
+u64_stats_update_end_irqrestore(struct u64_stats_sync *syncp,
+ unsigned long flags)
+{
+#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
+ write_seqcount_end(&syncp->seq);
+ local_irq_restore(flags);
+#endif
+}
+
static inline void u64_stats_update_begin_raw(struct u64_stats_sync *syncp)
{
#if BITS_PER_LONG==32 && defined(CONFIG_SMP)
next reply other threads:[~2018-03-05 19:41 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-05 19:41 Eric Dumazet [this message]
2018-03-05 19:41 ` [PATCH net] net: usbnet: fix potential deadlock on 32bit hosts Eric Dumazet
-- strict thread matches above, loose matches on Subject: below --
2018-03-07 16:47 [net] " David Miller
2018-03-07 16:47 ` [PATCH net] " David Miller
[not found] <CGME20180227072602eucas1p28dcdba711b3153820bd409d838cc63bd@eucas1p2.samsung.com>
2018-02-27 7:26 ` inconsistent lock state with usbnet/asix usb ethernet and xhci Marek Szyprowski
2018-02-27 10:37 ` Oliver Neukum
2018-02-27 10:59 ` Marek Szyprowski
2018-02-27 14:07 ` Eric Dumazet
2018-02-27 14:07 ` Eric Dumazet
2018-02-27 14:42 ` Marek Szyprowski
2018-02-27 14:42 ` Marek Szyprowski
2018-02-27 15:09 ` Eric Dumazet
2018-02-27 15:09 ` Eric Dumazet
2018-02-27 15:13 ` Eric Dumazet
2018-02-27 15:13 ` Eric Dumazet
2018-02-27 16:07 ` Oliver Neukum
2018-02-27 16:07 ` Oliver Neukum
2018-03-05 7:45 ` Marek Szyprowski
2018-03-05 7:45 ` Marek Szyprowski
2018-03-05 11:46 ` Oliver Neukum
2018-03-05 11:46 ` Oliver Neukum
2018-03-05 19:09 ` Eric Dumazet
2018-03-05 19:09 ` Eric Dumazet
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=1520278873.109662.14.camel@gmail.com \
--to=eric.dumazet@gmail.com \
--cc=Dean_Jenkins@mentor.com \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=netdev@vger.kernel.org \
--cc=oneukum@suse.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.