Do not do this when holding any locks, esp. sleepable.

Also be aware that other threads get spurious EINTRs if sleeping in the slow syscalls.

This causes spurious EAGAIN returned to the caller sometimes. Why not return ERESTART?

I could drop allprison_lock before this, or move this test to the front of the system calls (especially considering the ERESTART stuff mentioned below).

Unless "do this" means the entire single-threaded section between here and and thread_single_end shouldn't be sleepable. If that's the case, I'll have to find a completely different strategy.

Does ERESTART make it to the kernel/user boundary? I couldn't tell from a quick look through.

For jail_attach and jail_attach_jd that would work. jail_set(JAIL_CREATE | JAIL_ATTACH) is more complicated since restarting the system call would fail on attempting to re-create the jail. The solution may be to make this test in kern_jail_set before starting anything.

Single threading should be done at the beginning of the syscall, if possible. After single threading is established, there is no limitation on the synchronization operations that the executing thread can perform. Almost no limitations, the existing shenanigans are related to threading and debugging, which should be not relevant there.

It makes it back to userspace, and the syscall instruction is re-issued.

I'm content for spurious EINTR to be the caller's problem. This situation is unlikely to happen in real code, and the main focus is to keep things out of an invalid state (including possible security concerns, though no specific issue has been noted).

I prefer that we annotate unlocked accesses like this using atomic(9): if (atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0. It's a signal to readers and concurrency sanitizers that the unlocked access is "safe", in this case because 1) P_HADTHREADS is never cleared once set, and 2) if we're the only thread in the process, then P_HADTHREADS isn't going to be toggled concurrently.

Same comment elsewhere.

You might point to the comment in kern_jail_set(). (Or use a helper function to wrap thread_single().)

Could you please use unique jail names for each test? Otherwise the tests can't run in parallel.

style(9) wants a newline after the return type, ditto below.

Most tests omit the whitespace between ATF_TC/ATC_TC_HEAD/ATF_TC_BODY. I think that makes them more readable.

I went with the help function route, which also neatens it up nicely. Each call has the single-line comment, and the function has the larger one that kern_jail_set has (a little more than that, actually).

If you introduce a local error variable, then PROC_UNLOCK() can be done only once:

error = 0;
if ((atomic_load_int(&p->p_flag) & P_HADTHREADS) != 0) {
        PROC_LOCK(p);
        if (thread_single(p, SINGLE_BOUNDARY))
                error = ERESTART;
        PROC_UNLOCK(p);
}
return (error);