Re: [patch V2 7/8] posix-cpu-timers: Prevent enqueueing when PF_EXITING is set

[Oleg Nesterov <oleg@redhat.com>]

I do not want to spam lkml, so let me say that all the previous changes
look good to me, feel free to add

Reviewed-by: Oleg Nesterov <oleg@redhat.com>


But I am confused by this patch, even if it looks correct to me too.

On 09/05, Thomas Gleixner wrote:
>
> @@ -684,7 +706,13 @@ static void arm_timer(struct k_itimer *t
>  	struct cpu_timer *ctmr = &timer->it.cpu;
>  	u64 newexp = cpu_timer_getexpires(ctmr);
>
> +	lockdep_assert_held(&p->sighand->siglock);
> +
>  	timer->it_status = POSIX_TIMER_ARMED;
> +
> +	if (unlikely(!task_can_enqueue_timer(p, clock_pid_type(timer->it_clock))))
> +		return;

I can't understand why does it check task_can_enqueue_timer() after
setting POSIX_TIMER_ARMED. This adds the new armed-but-not-enqueued state,
afaics.

I see nothing wrong, it seems that this can only affect __posix_cpu_timer_get()
which checks ->it_status, other code paths do not check ->it_status.

But I don't understand this code, so let me ask: is it on purpose? I mean,
is there any reason to set _ARMED unconditionally ?

Oleg.