Page MenuHomeFreeBSD

Simplify taskqgroup inititialization.
ClosedPublic

Authored by markj on Mar 25 2020, 8:54 PM.

Details

Summary

taskqgroup initialization was broken into two steps:

  1. allocate the taskqgroup structure, at SI_SUB_TASKQ
  2. initialize taskqueues, start taskqueue threads, enqueue "binder" tasks to bind threads to specific CPUs, at SI_SUB_SMP

There is some complexity to handle the case where code attaches tasks to
a queue before step 2, in which case step 2 must migrate tasks to their
intended queue. Moreover, tasks can't be enqueued until step 2 has
completed. This breaks setups where EARLY_AP_STARTUP is not defined
and we need to use an iflib-based driver to mount a network root
filesystem, since mountroot happens before SI_SUB_SMP in this case.

If we do all initialization and thread creation in step 1, and only
defer CPU binding to SI_SUB_SMP, then things become a lot simpler and
the aforementioned problem is fixed. This does mean that there exists a
window where taskqueue threads may run on a CPU other than that to which
they are supposed to be bound. I believe, but am not yet 100% certain,
that this does not affect code correctness.

The change basically splits initialization as described above. It also
removes some code to handle re-adjustment of taskqueue CPU bindings,
since that is unused. The change also lets us simplify iflib
initialization a bit.

Test Plan

Sean reported not being able to netboot an arm64 system with igb interfaces,
since he hit the assertion failure in em_if_timer(). With this patch we can mount
the root filesystem.

I also booted an amd64 system without EARLY_AP_STARTUP and verified that
igb interfaces still work. Verified that taskqueue thread bindings are still correct,
ditto for igb rx interrupt bindings.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

markj added subscribers: sbruno, emaste.

As a slight detour is there any plan in place to get rid of !EARLY_AP_STARTUP? Stabilization is helpful here especially for MFC, but the number of test targets with EAPS and !EAPS made this code overwhelmingly fragile throughout its life.

As a slight detour is there any plan in place to get rid of !EARLY_AP_STARTUP? Stabilization is helpful here especially for MFC, but the number of test targets with EAPS and !EAPS made this code overwhelmingly fragile throughout its life.

I'm not aware of any plan, but yes it would be best to implement that across the board. It creates a lot of complexity elsewhere. Looks like it is only implemented on i386, amd64 and riscv so far. I will try and find out what it would take to get it implemented on arm64. That leaves arm, powerpc and mips.

The goal is for all platforms to implement EARLY_AP_STARTUP and then to remove it as an option by having it be always-on behavior. When I tried it on arm64 on my little RPi I ran into an assertion failure early on due to IPIs not being usable soon enough. PowerPC requires some work to support early AP startup. I haven't really looked at MIPS at all.

Obviously, without this change, no IFLIB enabled adapter will netboot/nfsboot on non-x86 hardware. For now, this was used in the FreeBSD Cluster to restore service to a failed ARM64 machine.

bdragon added a subscriber: bdragon.

Confirmed on powerpc64 as well. With this patch, I can now netboot off of an iflib card, without it, I hit the assertion.

(Tested with a dual-port em(4) card on a Raptor Blackbird.)

I'd like to commit this on the weekend if there are no objections.

This revision was not accepted when it landed; it landed in state Needs Review.Mar 30 2020, 2:23 PM
This revision was automatically updated to reflect the committed changes.