Re: [patch V2 10/50] posix-cpu-timers: Handle SIGEV_NONE timers correctly in timer_get()

[Thomas Gleixner <tglx@linutronix.de>]

On Fri, Apr 12 2024 at 13:40, Eric W. Biederman wrote:
> Thomas Gleixner <tglx@linutronix.de> writes:
>> -	if (iv && (timer->it_requeue_pending & REQUEUE_PENDING))
>> +	if (iv && ((timer->it_requeue_pending & REQUEUE_PENDING) || sigev_none))
>>  		expires = bump_cpu_timer(timer, now);
>>  	else
>>  		expires = cpu_timer_getexpires(&timer->it.cpu);
>> @@ -809,11 +811,13 @@ static void __posix_cpu_timer_get(struct
>>  		itp->it_value = ns_to_timespec64(expires - now);
>>  	} else {
> Why not make this else condition?
> 	} else if (!sigev_none) {
> And not need to change the rest of the code?

Duh, yes.
  		/*
>> -		 * The timer should have expired already, but the firing
>> -		 * hasn't taken place yet.  Say it's just about to expire.
>> +		 * A single shot SIGEV_NONE timer must return 0, when it is
>> +		 * expired! Timers which have a real signal delivery mode
>> +		 * must return a remaining time greater than 0 because the
>> +		 * signal has not yet been delivered.
>>  		 */
>> -		itp->it_value.tv_nsec = 1;
>> -		itp->it_value.tv_sec = 0;
>> +		if (!sigev_none)
>> +			itp->it_value.tv_nsec = 1;
>
> Do you perhaps need a comment somewhere that itp is zeroed in
> do_timer_gettime?  The code now depends upon that for setting
> itp->it_value when it did not used to, making it look at first
> glance like you have created an uninitialized variable.
>
> Probably just something in the description of the change would be
> sufficient.

Fair enough.

Thanks,

        tglx