zed: a lock-based pickle indeed

[nabijaczleweli]

Motivation and Context

#11963

Description

Lock around fork() and in the SIGCHLD handler. The latter should always lock as soon as a child dies and, if Linux does in fact re-use zombie PIDs, always come before the next fork().

But if this doesn't work then I genuinely don't know if this is possible under UNIX or if it's just always gonna race like this. See commit message.

How Has This Been Tested?

It wasn't. I mean, it runs, but. See commit message.

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)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil 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.

[nabijaczleweli]

Rebased, replaced with a solution that I tried, understood, and tested. @don-brady mind giving'er a spin?

If you can afford to do it, setting max_pids to something hilarious like 400 greatly exacerbates this, I hit it every time within minutes, if that.

[behlendorf]

@nabijaczleweli thanks for running this down.

[don-brady]

We tried working around this by launching zed with -j 1 and that was still hitting the assert. I have yet to test with this change but wanted to understand why having jobs set to 1 would allow async execution?

[nabijaczleweli]

-j guarantees that at any given point j_N - ∑(fork() ≠ -1) + ∑(wait4() ∉ {-1, 0}) >= 0, as documented.

The reaping methodology is unaffected, and the sequence in that case remains limit>0 => fork()=A => A:exit_group() => reaper:SIGCHLD => reaper:lock => limit-=1 => lock:sleep => reaper:wait4()=A => reaper:find(A)=NULL => reaper:unlock => limit+=1 => lock:wake => add(A) => unlock. This can then repeat with another child with PID of A, panicking on the add(A) step, and, since the limit ends at the same value that it started, you can see how it plays no role in this.

The updated sequence would look like limit>0 => lock => fork()=A => A:exit_group() => reaper:SIGCHLD => reaper:lоck:sleep() => add(A) => unlock => limit-=1 => reaper:lоck:wake => reaper:wait4()=A => reaper:find(A)=A* => reaper:unlock => limit+=1.