Hi, Eric: I have a couple comments inlined. >> On Wed, Oct 24, 2018 at 3:30 PM Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: >>> Enke Chen <enkechen@xxxxxxxxx> writes: >>>> For simplicity and consistency, this patch provides an implementation >>>> for signal-based fault notification prior to the coredump of a child >>>> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can >>>> be used by an application to express its interest and to specify the >>>> signal (SIGCHLD or SIGUSR1 or SIGUSR2) for such a notification. A new >>>> signal code (si_code), CLD_PREDUMP, is also defined for SIGCHLD. >>>> >>>> Changes to prctl(2): >>>> >>>> PR_SET_PREDUMP_SIG (since Linux 4.20.x) >>>> Set the child pre-coredump signal of the calling process to >>>> arg2 (either SIGUSR1, or SIUSR2, or SIGCHLD, or 0 to clear). >>>> This is the signal that the calling process will get prior to >>>> the coredump of a child process. This value is cleared across >>>> execve(2), or for the child of a fork(2). >>>> >>>> When SIGCHLD is specified, the signal code will be set to >>>> CLD_PREDUMP in such an SIGCHLD signal. >> [...] >>> Ugh. Your test case is even using signalfd. So you don't even want >>> this signal to be delivered as a signal. >> >> Just to make sure everyone's on the same page: You're suggesting that >> it might make sense to deliver the pre-dump notification via a new >> type of file instead (along the lines of signalfd, timerfd, eventfd >> and so on)? > > My real complaint was that the API was not being tested in the way it > is expected to be used. Which makes a test pretty much useless as some > aspect userspace could regress and the test would not notice because it > is testing something different. > > As I stated in a prior email, I have test code for both sigaction/waipid(), and signefd(). As the sigaction/waitpid is more widely used and that is what you prefer, I will change the selftest code to reflect that in the next version. Actually I should separate out the selftest code. > > I do think that a file descriptor based API might be a good alternative > to a signal based API. The proc connector and signals are not the only > API solution. > > The common solution to this problem is that distributions defailt the > rlimit core file size to 0. We do need coredumps in order to have the bugs fixed. Thanks. -- Enke
