| Message ID | 20190927154102.GA117350@ArchLaptop (mailing list archive) |
|---|---|
| State | Rejected |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | net: mac80211: Disable preeemption when updating stat counters | expand |
On Fri, 2019-09-27 at 11:41 -0400, Aaron Hill wrote: > The mac80211 subsystem maintains per-cpu stat counters for receive and > transmit operations. Previously, preemption was not disabled when > updating these counters. This creates a race condition where two cpus > could attempt to update the same counters using non-atomic operations. > > This was causing a > 'BUG: using smp_processor_id() in preemptible [00000000] code' > message to be printed, along with a stacktrace. This was reported > in a few different places: > > * https://www.spinics.net/lists/linux-wireless/msg189992.html > * https://bugzilla.kernel.org/show_bug.cgi?id=204127 > > This patch adds calls to preempt_disable() and preempt_enable() > surrounding the updating of the stat counters. That seems like basically the same as what Jiri reported, but now I'm even more confused... Ah. CONFIG_PREEMPT_RCU... But if we keep BHs disabled, it should still be OK, so what I suggested to Jiri will also address this I think? diff --git a/net/mac80211/util.c b/net/mac80211/util.c index 051a02ddcb85..ad1e88958da2 100644 --- a/net/mac80211/util.c +++ b/net/mac80211/util.c @@ -273,9 +273,9 @@ static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac) &txqi->flags)) continue; - spin_unlock_bh(&fq->lock); + spin_unlock(&fq->lock); drv_wake_tx_queue(local, txqi); - spin_lock_bh(&fq->lock); + spin_lock(&fq->lock); } } johannes
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index 768d14c9a716..5ef0667151bf 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -34,12 +34,17 @@ static inline void ieee80211_rx_stats(struct net_device *dev, u32 len) { - struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats); + struct pcpu_sw_netstats *tstats; + + // Disable preemption while updating per-cpu stats counters + preempt_disable(); + tstats = this_cpu_ptr(dev->tstats); u64_stats_update_begin(&tstats->syncp); tstats->rx_packets++; tstats->rx_bytes += len; u64_stats_update_end(&tstats->syncp); + preempt_enable(); } static u8 *ieee80211_get_bssid(struct ieee80211_hdr *hdr, size_t len, diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 1fa422782905..4cad3d741b6b 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -40,12 +40,17 @@ static inline void ieee80211_tx_stats(struct net_device *dev, u32 len) { - struct pcpu_sw_netstats *tstats = this_cpu_ptr(dev->tstats); + struct pcpu_sw_netstats *tstats; + + // Disable preemption while updating per-cpu stats counters + preempt_disable(); + tstats = this_cpu_ptr(dev->tstats); u64_stats_update_begin(&tstats->syncp); tstats->tx_packets++; tstats->tx_bytes += len; u64_stats_update_end(&tstats->syncp); + preempt_enable(); } static __le16 ieee80211_duration(struct ieee80211_tx_data *tx,
The mac80211 subsystem maintains per-cpu stat counters for receive and transmit operations. Previously, preemption was not disabled when updating these counters. This creates a race condition where two cpus could attempt to update the same counters using non-atomic operations. This was causing a 'BUG: using smp_processor_id() in preemptible [00000000] code' message to be printed, along with a stacktrace. This was reported in a few different places: * https://www.spinics.net/lists/linux-wireless/msg189992.html * https://bugzilla.kernel.org/show_bug.cgi?id=204127 This patch adds calls to preempt_disable() and preempt_enable() surrounding the updating of the stat counters. Signed-off-by: Aaron Hill <aa1ronham@gmail.com> --- net/mac80211/rx.c | 7 ++++++- net/mac80211/tx.c | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-)