Re: [RFC PATCH 2/3] exec: don't wait for zombie threads with cred_guard_mutex held

[Oleg Nesterov <oleg@redhat.com>]

On 11/11, Cyrill Gorcunov wrote:
>
> Anyway while looking into patch I got wonder why
>
> +static int wait_for_notify_count(struct task_struct *tsk)
> +{
> +	for (;;) {
> +			return -EINTR;
> +		set_current_state(TASK_KILLABLE);
> +		if (!tsk->signal->notify_count)
> +			break;
>
> We have no any barrier here in fetching @notify_count? I mean updating
> this value is done under locks (spin or read/write) in turn condition
> test is a raw one. Not a big deal since set_current_state() and schedule()

Yes, so I think that, correctness-wise, this doesn't need additional barriers.

> but I've
> been a bit confused that we don't use some read_once here or something.

Yes, this needs READ_ONCE() to avoid the warnings from KCSAN. And in fact
this code was written with READ_ONCE() but I removed it before sending this
RFC.

I was going to do this later. I always forget how KCSAN works, IIUC I also
need to add WRITE_ONCE() into exit_notify() and __exit_signal() to make
KCSAN happy, even if ->notify_count is always updated under the lock.

Same for the "if (me->signal->group_exec_task == me)" check in begin_new_exec().

Right now I would like to know if this RFC (approach) makes any sense,
especially because 3/3 adds a user-visible change.

Oleg.