In D21875#478050, @markj wrote:

In D21875#477890, @graeme.jenkinson_cl.cam.ac.uk wrote:

In D21875#477846, @markj wrote:

This makes sense to me, thanks. I'll commit it in a day or two if no one else jumps in.

Cool. It's only a minor thing but we are increasingly relying on correct knowledge of the internals of DIF and DOF to do our work on distribution.

Is there a "Sponsored by:" tag you'd like added to the commit message?

In D21875#477846, @markj wrote:

This makes sense to me, thanks. I'll commit it in a day or two if no one else jumps in.

Confirmed that calling dtrace_trap_func from abort_fatal() is correct (my mistake).

In D9701#201579, @meloun-miracle-cz wrote:

In my example a malformed script results in a memory access to 0x02. The resulting translation fault brings the system down. This should not happen.

Yes, due to missing FAULT_TRAN_L[1][2]: case in dtrace_trap().

dtrace_trap() tests whether the system is in probe context, by checking the flag cpu_core[curcpu].cpuc_dtrace_flags & CPU_DTRACE_NOFAULT. If this is set the translation fault should not be handled by the normal code path, instead the DTrace error probe should fire and the DTrace script resume processing.

In the original code dtrace_trap() was called from abort_fatal(). abort_fatal() is never called for translation faults (or alignment faults). It would be possible to populate the aborts array with handlers that call dtrace_trap() infor L1 and L2 translation faults and alignment faults, but I think that this is a bit messy.

I don't agree here. All fatal traps ends in abort_fatal(), including all translation or alignment faults.
Kernel mode FAULT_TRAN_L[1][2] traps calls abort_fatal() at line 501.

Other architectures handling dtrace_trap() immediately on entering the trap handler (I'll expand on this point when replying to the other comment).

But other architectures doesn't emulates access or modify bits in SW. In peaks, quadcore ARM can easily get over 100k/sec of traps, filtered down (by pmap_fault()) to 3~5 k of 'real' ones.

But again, are you're really sure that calling dtrace probe for trap caused by swapped out page(for example) and thus fully covered by vm_fault() is right one?
In other words, can dtrace probe handle these 'false alarms' ?
In my opinion no, at least not on ARM.

In any case, I haven't a single problem if you put dtrace hook anywhere behind the line 352.

In D9701#201533, @meloun-miracle-cz wrote:

Can you be, please, little more specific for
"abort_handler - didn't call the dtrace_trap() function early enough to handle faults caused by DTrace " ?

Which faults are not handled if KDTRACE_HOOKS is on original location?
I'm only curious here, and my knowledge of DTRACE are very limited. But are you sure that DTRACE wants to see faults, possibly handled by vm_fault()? Or by pcb_onfault()... ?

Review comments addressed.