On 22.01.2026 11:10, Roger Pau Monné wrote:
> On Thu, Jan 22, 2026 at 10:28:51AM +0100, Jan Beulich wrote:
>> On 22.01.2026 10:18, Roger Pau Monné wrote:
>>> On Mon, Nov 17, 2025 at 03:39:30PM +0100, Jan Beulich wrote:
>>>> When this was added, the log message was updated correctly, but the zero
>>>> case was needlessly checked separately: hpet_broadcast_enter() had a zero
>>>> check added at the same time, while handle_hpet_broadcast() can't possibly
>>>> pass 0 here anyway.
>>>>
>>>> Fixes: 7145897cfb81 ("cpuidle: Fix for timer_deadline==0 case")
>>>> Signed-off-by: Jan Beulich <[email protected]>
>>>
>>> Acked-by: Roger Pau Monné <[email protected]>
>>
>> Thanks.
>>
>>> Similar to the previous commit, I wonder whether it would make sense
>>> to add an ASSERT_UNREACHABLE() if that error path is not reachable
>>> given the logic in the callers.
>>
>> That would mean
>>
>> if ( unlikely(expire < 0) )
>> {
>> printk(KERN_DEBUG "reprogram: expire <= 0\n");
>> return -ETIME;
>> }
>>
>> if ( unlikely(expire == 0) )
>> {
>> ASSERT_UNREACHABLE();
>> return -ETIME;
>> }
>>
>> which I fear I don't like (for going too far). Even
>>
>> if ( unlikely(expire <= 0) )
>> {
>> printk(KERN_DEBUG "reprogram: expire <= 0\n");
>> ASSERT(expire);
>> return -ETIME;
>> }
>>
>> I'd be uncertain about, as that needlessly gives 0 a meaning that isn't
>> required anymore in this function.
>
> Hm, OK, I was under the impression that both < 0 and 0 should never be
> passed by the callers. If expire == 0 is a possible input then I
> don't think the ASSERT() is that helpful.
Oh, so you were after
if ( unlikely(expire <= 0) )
{
printk(KERN_DEBUG "reprogram: expire <= 0\n");
ASSERT_UNREACHABLE();
return -ETIME;
}
(perhaps even with the printk() dropped)? That I could buy off on, as NOW()
is expected to only ever return valid (positive) s_time_t values.
Jan
