Looks like there's a second half to this fix that's also needed, upstream PR is at:

Superseded by D58721, which covers both fixes here on a rewritten common unwind path and adds the SR-IOV schema and core-offset refcount cases. Abandoning in favor of that.

In D58720#1348789, @gallatin wrote:

This is really cool. I had never know about fail(9)

In D58337#1344794, @kib wrote:

I do not see how this could be an option. Either there is no (bad) impact on the native umtxes, and then we should just change the constant, perhaps adding a comment explaining its origin and reasoning behind. Or there is the impact, but then you could check that the curproc ABI is linux, and use the proposed constant for Linux processes.

gate on abi

Fixed by D58506

I’m late to the party but this covers every issue I’ve run into except one dumb one that I’m not sure should live in vfs to begin with

If anyone runs into this in the future it was a git-arc issue resolved in 3d4a61a10bb7

Refreshed after the comment-cleanup rebase of the series.

Refreshed after the comment-cleanup rebase of the series.

Refreshed after the comment-cleanup rebase of the series.

Refreshed after the comment-cleanup rebase of the series.

Refreshed after the comment-cleanup rebase of the series.

Collapse the multi-line block comments to single lines; the reasoning is already in the commit message.

Collapse the multi-line block comments to single lines; the reasoning is already in the commit message.

Collapse the multi-line block comments to single lines; the reasoning is already in the commit message.

Collapse the multi-line block comments to single lines; the reasoning is already in the commit message.

Trim the two block comments that restated commit-message paragraphs.

Add the Differential Revision trailer and drop the explanatory block comments; the reasoning is in the commit message.

Drop the explanatory block comments; the reasoning is in the commit message.

Gate the driver's own link UP/DOWN messages behind bootverbose, as ix and igc do, since if_link_state_change() already announces the transition and the pair was reported twice. The retrain speed line stays unconditional: a retrain holds the link up, so it is not a state change and nothing else reports it.

Rebased onto the updated parent; this commit's content is unchanged.

Rebased onto the updated parent; this commit's content is unchanged.

Rebased onto the updated parent; this commit's content is unchanged.

Quieten the default boot: gate the per-vector attach messages behind bootverbose, announce the Atlantic 2 firmware banner once per attach rather than on every bring-up, and say that a TX ring shares an RX ring's vector instead of reporting it as a separate assignment.

In D58178#1341543, @pho wrote:

In D58178#1341511, @nick_spun.io wrote:

Here are two new clean reproducers for this panic - there is also another panic doing mount_nullfs -o mountdir $(mktemp) (using -o mountdir on top of a file) but that is outside of this scope.

filemount_panic.sh1 KBDownload

filemount_panic.c3 KBDownload

I'm not seeing these attachment. Could you please email them to me?

Here are two new clean reproducers for this panic - there is also another panic doing mount_nullfs -o mountdir $(mktemp) (using -o mountdir on top of a file) but that is outside of this scope.

Apologies, the repro cases I posted previously need to be reworked a bit

Self-review pass, and three behavioural corrections. Keep the
generated-address fallback in aq_hw_get_mac_permanent() rather than
failing the attach, so a transient mailbox timeout cannot turn into a
missing interface; report the failure instead of substituting silently.
Keep aq_hw_init() going when the firmware link-mode acknowledgement times
out, since returning early skipped the interrupt-mode and IRQ-map
programming that follow. Retry the fw1x mailbox poll instead of returning
on the first error, which is what the surrounding 1000-iteration loop
exists to do while the MCP cold-starts. Also track the outstanding reset
so the init retry does not run one extra stop/init after the interface has
already recovered, and reset the retry budget per bring-up.

Self-review pass: no functional change to this commit. Commit message
amended to carry the Differential Revision reference.

Self-review pass. Do not let one PHY fault code mask another: record the
last code reported rather than latching a single seen flag, so a
non-thermal fault raised while the link is down no longer suppresses a
later thermal shutdown. Require a minimum interval between recovery
attempts, seeded at attach, since recovery costs a PHY reset plus a full
re-init and renegotiation. Probe the MDIO port address from every
consumer, not only thermal_arm(). Rename caps_hi bit 21 to
CAPS_HI_THERMAL_SHUTDOWN (naming only; nothing reads it).

Self-review pass: no functional change to this commit. Commit message
amended to carry the Differential Revision reference.

Confirmed today (by accidentally removing this patch from my local tree) that this is still required 😆️

Fold in the firmware/MDIO error-handling fixes from a second review round; they touch the same functions as this change.

Fold in the queue-allocation attach-failure leak; both fixes are on the same iflib_device_register() failure ladder.

Tighten source comments; no functional change.

Tighten source comments; no functional change.

Superseded by a consolidated aq(4) series; abandoning in favor of new revisions.

Superseded by a consolidated aq(4) series; abandoning in favor of new revisions.

Superseded by a consolidated aq(4) series; abandoning in favor of new revisions.

Superseded by a consolidated aq(4) series; abandoning in favor of new revisions.

In D58353#1338551, @diizzy wrote:

Thanks for looking into this!
I'm asking because I've seen countless of people reporting these controllers (especially on motherboards) as being notoriously unstable/flakey likely due to heat and the cause might not be obvious for the end user given that you normally have no issues at least in terms of heat with integrated NICs. I've also seen some reports on forums of the controller completely die as in drop off PCIe bus and never come back again but I'm not sure if that's only due to overheating. I agree that it probably should go into some kind of sensor/health framework in the end but it would be nice to some kind of warning for diagnostic purposes. While we can discuss thresholds later I'm not sure if I would consider it "safe" for it to go all up to 108C before shutting down/throttling, perhaps warn around 70C and kick in around 80C.

Merged upstream at https://github.com/openzfs/zfs/commit/d98fa72ca05a903710fd5ab132ee508c01280af0

In D58353#1338551, @diizzy wrote:

Thanks for looking into this!
I'm asking because I've seen countless of people reporting these controllers (especially on motherboards) as being notoriously unstable/flakey likely due to heat and the cause might not be obvious for the end user given that you normally have no issues at least in terms of heat with integrated NICs. I've also seen some reports on forums of the controller completely die as in drop off PCIe bus and never come back again but I'm not sure if that's only due to overheating. I agree that it probably should go into some kind of sensor/health framework in the end but it would be nice to some kind of warning for diagnostic purposes. While we can discuss thresholds later I'm not sure if I would consider it "safe" for it to go all up to 108C before shutting down/throttling, perhaps warn around 70C and kick in around 80C.

Looks like by default aq1 and aq2 PHYs will shut down at 108C and set a bit until they recover to a specified threshold

In D58353#1338352, @diizzy wrote:

Given the rather hot nature of these chips perhaps add some kind of temperature warning or possibly drop link speed (if it helps reducing temperature) once it reaches a threshold rather than it completely wedges itself?