> 在 2015年10月23日，17:55，Arnd Bergmann <arnd@xxxxxxxx> 写道：
>
> On Friday 23 October 2015 17:24:59 WEN Pingbo wrote:
>> Using struct timeval will cause time overflow in 2038, replacing it with
>> ktime_t. And we don't need to handle sec and nsec separately.
>>
>
> This part looks good now, but as I commented in version 1, it should
> really be a separate patch rather than combined with the rest that
> is dealing with another use of timeval.
Ok, I will split it in next version.
>> - do_gettimeofday(&tv);
>> - tv.tv_usec += USEC_PER_SEC * (tv.tv_sec - mlc->instart.tv_sec);
>> - tv.tv_usec -= mlc->instart.tv_usec;
>> - if (tv.tv_usec >= mlc->intimeout) goto sched;
>> - tv.tv_usec = (mlc->intimeout - tv.tv_usec) * HZ / USEC_PER_SEC;
>> - if (!tv.tv_usec) goto sched;
>> - mod_timer(&hil_mlcs_kicker, jiffies + tv.tv_usec);
>> + if (tmp.tv64 >= (mlc->intimeout * NSEC_PER_USEC))
>> + goto sched;
>> + tmp.tv64 = mlc->intimeout * NSEC_PER_USEC - tmp.tv64;
>> + if (tmp.tv64 < NSEC_PER_USEC)
>> + goto sched;
>> + mod_timer(&hil_mlcs_kicker,
>> + jiffies + nsecs_to_jiffies(tmp.tv64));
>> break;
>> sched:
>> tasklet_schedule(&hil_mlcs_tasklet);
>
> If I read this right, the code is executed one for each input event such
> as a keypress or mouse movement. In the latter case, doing nsecs_to_jiffies()
> here is actually a bit expensive, and I stil think it can be avoided
> by just using jiffies.
>
> For the (tmp.tv64 < NSEC_PER_USEC) part, did you just do that because
> I said this, or did you actually prove that it is required? I'm still
> confused about what the driver is trying to achieve here.
More explanation here:)
the judgement here is to prevent mod_timer with zero delta. I can not make sure whether the module
have nanosecond precise, so just keep same.
>
>> diff --git a/drivers/input/serio/hp_sdc_mlc.c b/drivers/input/serio/hp_sdc_mlc.c
>> index d50f067..0a27b89 100644
>> --- a/drivers/input/serio/hp_sdc_mlc.c
>> +++ b/drivers/input/serio/hp_sdc_mlc.c
>> @@ -149,7 +149,8 @@ static int hp_sdc_mlc_in(hil_mlc *mlc, suseconds_t timeout)
>>
>> /* Try to down the semaphore */
>> if (down_trylock(&mlc->isem)) {
>> - struct timeval tv;
>> + ktime_t tmp = ktime_sub(ktime_get(), mlc->instart);
>> +
>> if (priv->emtestmode) {
>> mlc->ipacket[0] =
>> HIL_ERR_INT | (mlc->opacket &
>
> Maybe give the variable a more useful name than 'tmp'?
>
> You could also remove the variable entirely if you slightly restructure
> the condition below.
I see, thanks for point out.
Pingbo
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
