Source-Changes-D archive

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

Re: CVS commit: src/sys





On 10/08/2024 22:37, Taylor R Campbell wrote:
Module Name:    src
Committed By:   skrll
Date:           Sat Aug 10 12:16:47 UTC 2024

Modified Files:
         src/sys/arch/arm/altera: cycv_gmac.c
         src/sys/arch/arm/amlogic: meson_dwmac.c
         src/sys/arch/arm/rockchip: rk_gmac.c
         src/sys/arch/arm/sunxi: sunxi_gmac.c
         src/sys/dev/ic: dwc_gmac.c dwc_gmac_var.h

Log Message:
awge(4): MP improvements

Remove the non-MP-safe scaffolding and make the locking less
coarse.

It's great to see more MP-safety and removal of conditional-NET_MPSAFE
scaffolding -- but please, no more all-encompassing `core locks'!

The usage model for a network interface is:

1. (under IFNET_LOCK) if_init:
    (a) resets hardware, while nothing else is touching registers
    (b) programs multicast filter
    (c) starts Tx/Rx and tick for mii/watchdog
    (d) sets IFF_RUNNING

2. concurrently:
    - (mii lock) periodic mii ticks or (with IFNET_LOCK too) ifmedia ioctls
    - (tx lock) Tx submissions
    - (rx lock) Rx notifications
    - (multicast filter lock) SIOCADDMULTI/SIOCDELMULTI
    - (ic lock) 802.11 state machine notifications
    - (IFNET_LOCK) maybe other ioctls (some of which return ENETRESET
      to cause an if_stop/init cycle in thread context)

3. (under IFNET_LOCK) if_stop:
    (a) clears IFF_RUNNING
    (b) halts Tx/Rx/tick and waits for completion
    (c) disables concurrent multicast updates
    (d) calls mii_down to wait for concurrent mii activity to quiesce
    (e) resets hardware, now that nothing is touching registers any more

All of the steps in (1) if_init, and all of the steps in (3) if_stop,
are already serialized by IFNET_LOCK, the heavyweight configuration
lock, in low-priority thread context.  Sometimes, while (2) running,
ifconfig(8) will reconfigure things, also serialized by IFNET_LOCK.

So there's _no need_ for a `core lock' to cover everything in if_init
and if_stop: IFNET_LOCK already takes care of that.

Any high-priority activity like Tx/Rx paths, callouts, &c., MUST NOT
take IFNET_LOCK or wait for the completion of the heavyweight
configuration changes, which often have to wait for concurrent
high-priority activity to cease -- such waits lead to deadlocks like
<https://mail-index.netbsd.org/tech-net/2022/01/10/msg008170.html>.

So it's _harmful_ to have a `core lock' cover everything in if_init
and if_stop.

Thanks for keeping me straight.

Is this documented in sys/net or somewhere else in the source tree?

Nick


Home | Main Index | Thread Index | Old Index