Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev/pci rxq_ptr must be updated holding rxq_lock consist...



details:   https://anonhg.NetBSD.org/src/rev/2327c25019a3
branches:  trunk
changeset: 821803:2327c25019a3
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Fri Feb 17 11:57:26 2017 +0000

description:
rxq_ptr must be updated holding rxq_lock consistent with the Rx processing.

In previous implementation, wm_rxeof() temporarily unlock rxq->rxq_lock to
call if_percpuq_enqueue(), and then re-lock rxq->rxq_lock to update
rxq->rxq_ptr. So, if multiple CPUs share the same rxq, there is race e.g.
    - CPU A: lock rxq->rxq_lock
    - CPU A: Rx processing include increment local variable "i"
    - CPU A: unlock rxq->rxq_lock
    - CPU A: call if_percpuq_enqueue()
    - CPU B: lock rxq->rxq_lock
    - CPU B: Rx processing include increment local variable "i"
    - CPU B: unlock rxq->rxq_lock
    - CPU B: call if_percpuq_enqueue()
    - CPU B: lock rxq->rxq_lock
    - CPU B: update rxq->rxq_ptr, that is, set CPU B's local "i" to rxq->rxq_ptr
    - CPU B: unlock rxq->rxq_lock
    - CPU A: lock rxq->rxq_lock
    - CPU A: update rxq->rxq_ptr, that is, set CPU A's local "i" to rxq->rxq_ptr
    - CPU A: unlock rxq->rxq_lock

diffstat:

 sys/dev/pci/if_wm.c |  22 +++++++++++++++-------
 1 files changed, 15 insertions(+), 7 deletions(-)

diffs (57 lines):

diff -r bd64be0ff96c -r 2327c25019a3 sys/dev/pci/if_wm.c
--- a/sys/dev/pci/if_wm.c       Fri Feb 17 11:23:21 2017 +0000
+++ b/sys/dev/pci/if_wm.c       Fri Feb 17 11:57:26 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_wm.c,v 1.482 2017/02/17 07:21:28 knakahara Exp $    */
+/*     $NetBSD: if_wm.c,v 1.483 2017/02/17 11:57:26 knakahara Exp $    */
 
 /*
  * Copyright (c) 2001, 2002, 2003, 2004 Wasabi Systems, Inc.
@@ -84,7 +84,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.482 2017/02/17 07:21:28 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_wm.c,v 1.483 2017/02/17 11:57:26 knakahara Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_net_mpsafe.h"
@@ -7799,8 +7799,14 @@
                uint8_t rsstype = wm_rxdesc_get_rsstype(rxq, i);
 #endif
 
-               if (!wm_rxdesc_dd(rxq, i, status))
-                       break;
+               if (!wm_rxdesc_dd(rxq, i, status)) {
+                       /*
+                        * Update the receive pointer holding rxq_lock
+                        * consistent with increment counter.
+                        */
+                       rxq->rxq_ptr = i;
+                       break;
+               }
 
                count++;
                if (__predict_false(rxq->rxq_discard)) {
@@ -7924,7 +7930,11 @@
 
                /* Set up checksum info for this packet. */
                wm_rxdesc_ensure_checksum(rxq, status, errors, m);
-
+               /*
+                * Update the receive pointer holding rxq_lock consistent with
+                * increment counter.
+                */
+               rxq->rxq_ptr = i;
                mutex_exit(rxq->rxq_lock);
 
                /* Pass it on. */
@@ -7936,8 +7946,6 @@
                        break;
        }
 
-       /* Update the receive pointer. */
-       rxq->rxq_ptr = i;
        if (count != 0)
                rnd_add_uint32(&sc->rnd_source, count);
 



Home | Main Index | Thread Index | Old Index