From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753352AbbJWMc5 (ORCPT ); Fri, 23 Oct 2015 08:32:57 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:36633 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752644AbbJWMcy convert rfc822-to-8bit (ORCPT ); Fri, 23 Oct 2015 08:32:54 -0400 Content-Type: text/plain; charset=gb2312 Mime-Version: 1.0 (Mac OS X Mail 9.1 \(3096.5\)) Subject: Re: [Y2038] [PATCH V2] hil_mlc: convert timeval to ktime_t From: Pingbo Wen In-Reply-To: <33014393.CcP7q7PhGq@wuerfel> Date: Fri, 23 Oct 2015 20:32:50 +0800 Cc: y2038@lists.linaro.org, dmitry.torokhov@gmail.com, linux-kernel@vger.kernel.org, linux-input@vger.kernel.org Content-Transfer-Encoding: 8BIT Message-Id: <05CEA4E0-A8CF-42DA-89C3-F0968DE73CB4@linaro.org> References: <46052377.96MapaKFAy@wuerfel> <1445592300-30894-1-git-send-email-pingbo.wen@linaro.org> <33014393.CcP7q7PhGq@wuerfel> To: Arnd Bergmann X-Mailer: Apple Mail (2.3096.5) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > 在 2015年10月23日，17:55，Arnd Bergmann 写道： > > 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