Re: [PATCH] posix-timers: Handle returned errors poperly in [i]timer_delete()

[Anna-Maria Behnsen]

Oleg Nesterov <oleg@xxxxxxxxxx> writes:

> On 04/15, Anna-Maria Behnsen wrote:
>>
>> timer_delete_hook() returns -EINVAL when the clock or the timer_del
>> callback of the clock does not exist. This return value is not handled by
>> the callsites timer_delete() and itimer_delete().
>
> IIUC this shouldn't happen? timer_delete_hook() WARN()s in this case,
> not sure we need to return this error to userspace...

This shouldn't happen, right.

Even if we do not return this error to userspace, is it valid to proceed
with the rest of the callsites? When it is fine to just ignore the
-EINVAL return, then I would propose just to add a comment to the code.

>> --- a/kernel/time/posix-timers.c
>> +++ b/kernel/time/posix-timers.c
>> @@ -1009,6 +1009,7 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t
>>  {
>>  	struct k_itimer *timer;
>>  	unsigned long flags;
>> +	int ret;
>>  
>>  	timer = lock_timer(timer_id, &flags);
>>  
>> @@ -1019,7 +1020,11 @@ SYSCALL_DEFINE1(timer_delete, timer_t, t
>>  	/* Prevent signal delivery and rearming. */
>>  	timer->it_signal_seq++;
>>  
>> -	if (unlikely(timer_delete_hook(timer) == TIMER_RETRY)) {
>> +	ret = timer_delete_hook(timer);
>> +	if (ret < 0)
>> +		return ret;
>
> unlock_timer() ?
>

bah... was done in a hurry...

Thanks,

	Anna-Maria