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.

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 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?

Based on your description of the problem, the current use of MOD_UNLOAD is correct, and you can forget my suggestion about MOD_QUIESCE. As done here, refusing to unload with samples in-flight is the safest course of action. The extended errors will inform the user precisely why unload fails.