Which file triggered this problem?

Confirmed with jhb while at the conference, he's good with it. I've also done a tinderbox build with this patchset and it looks good on all of the kernel compiles.

Thanks, fair points. Some context before I change anything:

In D60981#1405620, @ojanerif_amd.com wrote:

The new directory needs an entry in etc/mtree/BSD.tests.dist (under usr.sbin, before certctl). With that, the tests install and pass here. The OptionalObsoleteFiles.inc entries ngie asked for are still missing too.

In D61320#1405753, @zlei wrote:

In D61320#1405672, @adrian wrote:

is m_len / m_pktlen ever supposed to be negative?

They should not be negative. But buggy driver / net stack logic may lead to. Here is an example 6429199a2113 (llan: byte swap the receive queue entries) .

In D61320#1405672, @adrian wrote:

is m_len / m_pktlen ever supposed to be negative?

I don't trust this fully.

• Unsupported arch: s/atf_fail/atf_skip/.
• Update OptionalObsoleteFiles.inc.
• Update etc/mtree/BSD.tests.dist.

I like this approach better. We do need to eventually close any if_baudrate driver gaps but the new function limits prevent that from blocking progress here.

Simplify somewhat.

Expand to test both traditional and nodup cases.

This new array allocation is not necessary.

Please see my previous comment.

The commit title is missing, it's just iscsi:.

In D61315#1405674, @kib wrote:

In D61315#1405673, @markj wrote:

Can't we just set NOCAPCHECK in NDINIT_ATVP?

Then it would allow dotdot. Even if it is safe now, it feels like a landmine for some future use.

LGTM

The hook is needed, but I think it has to run before thread_unlock(),
not after.

In D60561#1404506, @afscoelho_gmail.com wrote:

In D60561#1404441, @mhorne wrote:

What is the consequence here, can the unload race cause a panic or memory leak? Or is it purely functional (lost samples)?

I ask because, there is an additional module state, MOD_QUIESCE. It immediately precedes MOD_UNLOAD in the linker_file_unload() path, and functions similarly, but its errors can be overridden by force (kldunload -f / LINKER_UNLOAD_FORCE).
MOD_QUIESCE might be the appropriate state for this cleanup.

See module(9), kldunload(2), kldunload(8) man pages for more information.

Thanks, the problem is more than lost samples. It is that a queued sample keeps a reference to its PMC until we process it.
Cleanup disables the hooks that process those samples, then waits for the references to reach zero. If samples remain in the queue, that wait can last forever. With INVARIANTS, it can panic with “waiting too long for pmc to be free” instead.
Yeah, the owner check is conservative, but I think it avoids this unsafe cleanup. Couldn't we have an issue moving the check to MOD_QUIESCE and then let -f proceed with samples still queued ? Couldn't cleanup disable sample processing before those samples release their PMC reference?

In D61315#1405673, @markj wrote:

Can't we just set NOCAPCHECK in NDINIT_ATVP?

Can't we just set NOCAPCHECK in NDINIT_ATVP?

is m_len / m_pktlen ever supposed to be negative?

@wulf I have a ports committer bit. Should I push to the src since I have your approval?

In D56143#1405556, @bcr wrote:

Do we need to update this patch further or can it be committed as it is?

installworld fails with this applied:

This no longer compiles on main:

Move status_loc initialization to ps5ds_attach()