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

[Thomas Gleixner]

On Sun, Sep 06 2026 at 18:26, Oleg Nesterov wrote:
> 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 ?

It is intentional.

The problem is that timer_create(2), timer_settime(2), timer_gettime(2)
and timer_delete(2) are "functional" today as long as a task is visible,
i.e. hashed. By some definition of functional.

Since f90fff1e152d ("posix-cpu-timers: fix race between
handle_posix_cpu_timers() and posix_cpu_timer_del()") an exiting task
does not expire timers anymore. That commit used task->exit_state, which
is set way after exit_signals() in do_exit(). I made that earlier in the
previous commit.

So while timers still can be [re-]armed they won't expire which means
that queueing them in the first place is pointless. But I kept the state
modification to avoid behavioural changes. The current state is that a
timer is "armed" until it expired and the signal is queued. I just
preserved that.

Of course one could argue that once a task reached PF_EXITING, timers
armed on them don't matter anymore and simply return -ESRCH, but that's
a user visible change which I wanted to avoid right now.

I'm happy to stop pretending that this "works" after PF_EXITING is set,
but don't have strong opinions either.

Thanks,

        tglx