Re: [PATCH] fs: limit maximum concurrent coredumps

[Oleg Nesterov <oleg@redhat.com>]

On 06/21, Edward Allcutt wrote:
>
> The ability to limit concurrent coredumps allows dumping core to be safely
> enabled in these situations without affecting responsiveness of the system
> as a whole.

OK, but please note that the patch is not right,

> @@ -1844,6 +1845,7 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
>  	int retval = 0;
>  	int flag = 0;
>  	int ispipe;
> +	int dump_count = 0;
>  	static atomic_t core_dump_count = ATOMIC_INIT(0);
>  	struct coredump_params cprm = {
>  		.signr = signr,
> @@ -1865,6 +1867,14 @@ void do_coredump(long signr, int exit_code, struct pt_regs *regs)
>  	if (!__get_dumpable(cprm.mm_flags))
>  		goto fail;
>
> +	dump_count = atomic_inc_return(&core_dump_count);
> +	if (core_max_concurrency && (core_max_concurrency < dump_count)) {
> +		printk(KERN_WARNING "Pid %d(%s) over core_max_concurrency\n",
> +		       task_tgid_vnr(current), current->comm);
> +		printk(KERN_WARNING "Skipping core dump\n");
> +		goto fail;
> +	}
> +

We can't return here. We should kill other threads which share the same
->mm in any case.

Suppose that core_dump_count > core_max_concurrency, and we send, say,
SIGQUIT to the process. With this patch SIGQUIT suddenly starts to kill
the single thread, this must not happen.

If you change the patch to sleep until core_dump_count < core_max_concurrency,
then, again, we should kill other threads first.

Oleg.