From: Simon Horman <horms@kernel.org>
To: Mirsad Todorovac <mirsad.todorovac@alu.unizg.hr>
Cc: Heiner Kallweit <hkallweit1@gmail.com>,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
nic_swsd@realtek.com, "David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Marco Elver <elver@google.com>
Subject: Re: [PATCH v2 3/3] r8169: fix the KCSAN reported data-race in rtl_tx while reading TxDescArray[entry].opts1
Date: Wed, 18 Oct 2023 15:56:28 +0200 [thread overview]
Message-ID: <20231018135628.GQ1940501@kernel.org> (raw)
In-Reply-To: <992dcaf7-2b24-4e91-8c69-a5471da209ae@alu.unizg.hr>
On Tue, Oct 17, 2023 at 10:43:36PM +0200, Mirsad Todorovac wrote:
> On 10/17/23 22:01, Simon Horman wrote:
> > On Mon, Oct 16, 2023 at 11:47:56PM +0200, Mirsad Goran Todorovac wrote:
> > > KCSAN reported the following data-race:
> > >
> > > ==================================================================
> > > BUG: KCSAN: data-race in rtl8169_poll (drivers/net/ethernet/realtek/r8169_main.c:4368 drivers/net/ethernet/realtek/r8169_main.c:4581) r8169
> > >
> > > race at unknown origin, with read to 0xffff888140d37570 of 4 bytes by interrupt on cpu 21:
> > > rtl8169_poll (drivers/net/ethernet/realtek/r8169_main.c:4368 drivers/net/ethernet/realtek/r8169_main.c:4581) r8169
> > > __napi_poll (net/core/dev.c:6527)
> > > net_rx_action (net/core/dev.c:6596 net/core/dev.c:6727)
> > > __do_softirq (kernel/softirq.c:553)
> > > __irq_exit_rcu (kernel/softirq.c:427 kernel/softirq.c:632)
> > > irq_exit_rcu (kernel/softirq.c:647)
> > > sysvec_apic_timer_interrupt (arch/x86/kernel/apic/apic.c:1074 (discriminator 14))
> > > asm_sysvec_apic_timer_interrupt (./arch/x86/include/asm/idtentry.h:645)
> > > cpuidle_enter_state (drivers/cpuidle/cpuidle.c:291)
> > > cpuidle_enter (drivers/cpuidle/cpuidle.c:390)
> > > call_cpuidle (kernel/sched/idle.c:135)
> > > do_idle (kernel/sched/idle.c:219 kernel/sched/idle.c:282)
> > > cpu_startup_entry (kernel/sched/idle.c:378 (discriminator 1))
> > > start_secondary (arch/x86/kernel/smpboot.c:210 arch/x86/kernel/smpboot.c:294)
> > > secondary_startup_64_no_verify (arch/x86/kernel/head_64.S:433)
> > >
> > > value changed: 0xb0000042 -> 0x00000000
> > >
> > > Reported by Kernel Concurrency Sanitizer on:
> > > CPU: 21 PID: 0 Comm: swapper/21 Tainted: G L 6.6.0-rc2-kcsan-00143-gb5cbe7c00aa0 #41
> > > Hardware name: ASRock X670E PG Lightning/X670E PG Lightning, BIOS 1.21 04/26/2023
> > > ==================================================================
> > >
> > > The read side is in
> > >
> > > drivers/net/ethernet/realtek/r8169_main.c
> > > =========================================
> > > 4355 static void rtl_tx(struct net_device *dev, struct rtl8169_private *tp,
> > > 4356 int budget)
> > > 4357 {
> > > 4358 unsigned int dirty_tx, bytes_compl = 0, pkts_compl = 0;
> > > 4359 struct sk_buff *skb;
> > > 4360
> > > 4361 dirty_tx = tp->dirty_tx;
> > > 4362
> > > 4363 while (READ_ONCE(tp->cur_tx) != dirty_tx) {
> > > 4364 unsigned int entry = dirty_tx % NUM_TX_DESC;
> > > 4365 u32 status;
> > > 4366
> > > → 4367 status = le32_to_cpu(tp->TxDescArray[entry].opts1);
> > > 4368 if (status & DescOwn)
> > > 4369 break;
> > > 4370
> > > 4371 skb = tp->tx_skb[entry].skb;
> > > 4372 rtl8169_unmap_tx_skb(tp, entry);
> > > 4373
> > > 4374 if (skb) {
> > > 4375 pkts_compl++;
> > > 4376 bytes_compl += skb->len;
> > > 4377 napi_consume_skb(skb, budget);
> > > 4378 }
> > > 4379 dirty_tx++;
> > > 4380 }
> > > 4381
> > > 4382 if (tp->dirty_tx != dirty_tx) {
> > > 4383 dev_sw_netstats_tx_add(dev, pkts_compl, bytes_compl);
> > > 4384 WRITE_ONCE(tp->dirty_tx, dirty_tx);
> > > 4385
> > > 4386 netif_subqueue_completed_wake(dev, 0, pkts_compl, bytes_compl,
> > > 4387 rtl_tx_slots_avail(tp),
> > > 4388 R8169_TX_START_THRS);
> > > 4389 /*
> > > 4390 * 8168 hack: TxPoll requests are lost when the Tx packets are
> > > 4391 * too close. Let's kick an extra TxPoll request when a burst
> > > 4392 * of start_xmit activity is detected (if it is not detected,
> > > 4393 * it is slow enough). -- FR
> > > 4394 * If skb is NULL then we come here again once a tx irq is
> > > 4395 * triggered after the last fragment is marked transmitted.
> > > 4396 */
> > > 4397 if (READ_ONCE(tp->cur_tx) != dirty_tx && skb)
> > > 4398 rtl8169_doorbell(tp);
> > > 4399 }
> > > 4400 }
> > >
> > > tp->TxDescArray[entry].opts1 is reported to have a data-race and READ_ONCE() fixes
> > > this KCSAN warning.
> > >
> > > 4366
> > > → 4367 status = le32_to_cpu(READ_ONCE(tp->TxDescArray[entry].opts1));
> > > 4368 if (status & DescOwn)
> > > 4369 break;
> > > 4370
> > >
> > > Fixes: ^1da177e4c3f4 ("initial git repository build")
> >
> > Hi Mirsad,
> >
> > The fixes tag above seems wrong.
>
> Hi, Simon,
>
> It is taken directly from "git blame" as you can check for yourself.
>
> It is supposed to tag the commits prior to the introduction of git.
>
> If you have a better idea how to denote those, I will be happy to learn,
> but I have no better clue than what "git blame" gives ...
Interesting, thanks for the explanation.
I do think it's more usual, in such cases, to use the following.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Best regards,
> Mirsad Todorovac
> > > Cc: Heiner Kallweit <hkallweit1@gmail.com>
> > > Cc: nic_swsd@realtek.com
> > > Cc: "David S. Miller" <davem@davemloft.net>
> > > Cc: Eric Dumazet <edumazet@google.com>
> > > Cc: Jakub Kicinski <kuba@kernel.org>
> > > Cc: Paolo Abeni <pabeni@redhat.com>
> > > Cc: Marco Elver <elver@google.com>
> > > Cc: netdev@vger.kernel.org
> > > Link: https://lore.kernel.org/lkml/dc7fc8fa-4ea4-e9a9-30a6-7c83e6b53188@alu.unizg.hr/
> > > Signed-off-by: Mirsad Goran Todorovac <mirsad.todorovac@alu.unizg.hr>
> > > Acked-by: Marco Elver <elver@google.com>
> >
> > ...
>
next prev parent reply other threads:[~2023-10-18 13:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-16 21:47 [PATCH v2 1/3] r8169: fix the KCSAN reported data-race in rtl_tx() while reading tp->cur_tx Mirsad Goran Todorovac
2023-10-16 21:47 ` [PATCH v2 2/3] r8169: fix the KCSAN reported data race in rtl_rx while reading desc->opts1 Mirsad Goran Todorovac
2023-10-16 21:47 ` [PATCH v2 3/3] r8169: fix the KCSAN reported data-race in rtl_tx while reading TxDescArray[entry].opts1 Mirsad Goran Todorovac
2023-10-17 20:01 ` Simon Horman
2023-10-17 20:43 ` Mirsad Todorovac
2023-10-18 13:56 ` Simon Horman [this message]
2023-10-18 19:42 ` Mirsad Todorovac
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=20231018135628.GQ1940501@kernel.org \
--to=horms@kernel.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=elver@google.com \
--cc=hkallweit1@gmail.com \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mirsad.todorovac@alu.unizg.hr \
--cc=netdev@vger.kernel.org \
--cc=nic_swsd@realtek.com \
--cc=pabeni@redhat.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.