arc: add a few invariant checks in release builds

[andriytk]

Motivation and Context

There are these very-hard-to-reproduce kernel panics #18782 and #11338, so it would be nice to root-case them.

Description

Convert a couple ASSERTs invariants to VERIFYs to enforce them in release builds. They should help to understand the cause of the problem, whenever it happens again.

How Has This Been Tested?

CI tests here would be enough I guess.

Types of Changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Quality assurance (non-breaking change which makes the code more robust against bugs)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[amotin]

arc_evict_hdr() should not be called with I/O in progress, or when any reference is present (I/O also holds a references). It is asserted there. So the provided example does not sound real. The comment mention block free during I/O, not eviction...

[andriytk]

It is asserted there.

@amotin, I'm not sure those asserts are checked in release builds, are they?

The comment mention block free during I/O, not eviction...

Thanks for that. Just to clarify, you mean this comment, right?

         * not possible to evict it during the I/O.  The only possible
         * reason for it not to be found is if we were freed during the
         * read.

Do you mean that when block free happens hdr->b_l1hdr.b_state changes to arc_anon and that's why it may not be present in buf_hash? If so, it makes sense.

[amotin]

It is asserted there.

I'm not sure those asserts are checked in release builds, are they?

VERIFY() are built always. ASSERT() only in debug builds. But we can't assert everything production, that is why we have debug builds.

The comment mention block free during I/O, not eviction...

Thanks for that. Just to clarify, you mean this comment, right?

         * not possible to evict it during the I/O.  The only possible
         * reason for it not to be found is if we were freed during the
         * read.

Do you mean that when block free happens hdr->b_l1hdr.b_state changes to arc_anon and that's why it may not be present in buf_hash? If so, it makes sense.

Yes and yes, but I haven't looked on the free path recently.

[andriytk]

Last couple of days I almost cracked my head staring at the code, chatting with different AIs and trying to figure out a possible scenario that could lead to the bug - no luck. But it was a good exercise to become familiar with the code!

So yeah, converting a couple of ASSERTs to VERIFYs is the only thing we can do atm.

[behlendorf]

Yup, given how elusive this race has been promoting these checks to VERIFYs as a debugging change may result in some new evidence and insight.