DEV Community

Yutaka Hirata
Yutaka Hirata

Posted on

[3/4] A Shutdown Flag Is Only the Beginning

#linux #c #networking #opensource

Checking a shutdown flag and enabling interrupts are separate operations. Reading Intel's igb as two CPUs running in parallel exposes a check-and-enable gap that the flag alone does not exclude.

When reviewing a driver, I look for the signal that says: no more work. Stop new activity, wait for existing work, then clean up.

In the previous article, r8169's synchronize_net() changed my initial reading. Intel's igb Ethernet driver raised a different question in my local Linux 7.1.3 and 7.3 trees.

Between checking and doing

NAPI batches packet processing after an interrupt. When igb_poll() completes that work, it calls igb_ring_irq_enable(). The relevant excerpt is:

if (!test_bit(__IGB_DOWN, &adapter->state)) {
        if (adapter->flags & IGB_FLAG_HAS_MSIX)
                wr32(E1000_EIMS, q_vector->eims_value);
        else
                igb_irq_enable(adapter);
}
Enter fullscreen mode Exit fullscreen mode

Shutdown sets __IGB_DOWN, then later calls igb_irq_disable().

But checking the flag and enabling interrupts are separate operations.

CPU A can pass the check and enter the branch. While both CPUs run in parallel, CPU B can set the flag and mask interrupts before CPU A reaches the enable write. CPU A can then enable interrupts, undoing that mask. An atomic bit operation does not make the whole check-and-write sequence indivisible.

A possible check-and-enable interleaving

MSI-X uses multiple interrupt vectors. Its branch writes the enable register directly, so searching only for calls to igb_irq_enable() misses part of the picture.

What has actually been established?

igb_irq_disable() flushes register writes and calls synchronize_irq() to wait for interrupt handlers. Shutdown subsequently disables each NAPI instance. Neither wait, by itself, proves that a separate poll's final enable has finished. The NAPI documentation distinguishes ownership release from poll return.

This path has no synchronize_net(). NAPI does not mandate that pairing, but the flag check alone does not exclude this ordering. Poll running inside an RCU region does not automatically serialize shutdown with it: the shutdown path needs an actual synchronization step or another mechanism that excludes the race.

This is a source-review concern, not a reproduced crash. Shutdown also stops reception and transmission and later resets the hardware. Those details matter when determining the actual consequences.

Overnight load/unload tests remain useful. They do not promise to hit every narrow window. Sometimes it helps to read the code as two CPUs taking turns.

I discuss this kind of NAPI shutdown review in Getting Started with Linux Device Drivers, Volume 3. Quiet cleanup code deserves some attention, too. Someone will depend on it when their working day ends.

Was there code elsewhere that kept this gap closed? I went looking through the Linux kernel and found the pattern I wanted. It was in another Realtek driver, rtw88, this time for wireless hardware. How did it keep the flag check and interrupt enable together? That is where I head next.

Top comments (0)