From mboxrd@z Thu Jan 1 00:00:00 1970 From: Baolin Wang Subject: Re: [PATCH 11/11] k_clock:Remove the 32bit methods with timespec type Date: Mon, 20 Apr 2015 17:00:15 +0800 Message-ID: References: <1429509459-17068-1-git-send-email-baolin.wang@linaro.org> <1429509459-17068-12-git-send-email-baolin.wang@linaro.org> <20150420084229.GA4608@localhost.localdomain> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============7057139860070527656==" Cc: pang.xunlei@linaro.org, peterz@infradead.org, heiko.carstens@de.ibm.com, paulus@samba.org, cl@linux.com, heenasirwani@gmail.com, linux-arch@vger.kernel.org, linux-s390@vger.kernel.org, y2038 Mailman List , rafael.j.wysocki@intel.com, ahh@google.com, fweisbec@gmail.com, pjt@google.com, riel@redhat.com, Arnd Bergmann , schwidefsky@de.ibm.com, John Stultz , Thomas Gleixner , rth@twiddle.net, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, tj@kernel.org, linux390@de.ibm.com, linuxppc-dev@lists.ozlabs.org To: Richard Cochran Return-path: In-Reply-To: <20150420084229.GA4608@localhost.localdomain> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: linuxppc-dev-bounces+glppe-linuxppc-embedded-2=m.gmane.org@lists.ozlabs.org Sender: "Linuxppc-dev" List-Id: netdev.vger.kernel.org --===============7057139860070527656== Content-Type: multipart/alternative; boundary=047d7bfd05fc93d64405142428f5 --047d7bfd05fc93d64405142428f5 Content-Type: text/plain; charset=UTF-8 On 20 April 2015 at 16:42, Richard Cochran wrote: > On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote: > > > @@ -911,18 +907,14 @@ retry: > > return -EINVAL; > > > > kc = clockid_to_kclock(timr->it_clock); > > - if (WARN_ON_ONCE(!kc || (!kc->timer_set && !kc->timer_set64))) { > > + if (WARN_ON_ONCE(!kc || !kc->timer_set64)) { > > error = -EINVAL; > > } else { > > - if (kc->timer_set64) { > > - new_spec64 = itimerspec_to_itimerspec64(new_spec); > > - error = kc->timer_set64(timr, flags, &new_spec64, > > - &old_spec64); > > - if (old_setting) > > - old_spec = > itimerspec64_to_itimerspec(old_spec64); > > - } else { > > - error = kc->timer_set(timr, flags, &new_spec, rtn); > > - } > > + new_spec64 = itimerspec_to_itimerspec64(new_spec); > > + error = kc->timer_set64(timr, flags, &new_spec64, > > + &old_spec64); > > This statement can fit on one line. > > > + if (old_setting) > > + old_spec = itimerspec64_to_itimerspec(old_spec64); > > } > > > > unlock_timer(timr, flag); > > > @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, > which_clock, > > if (!kc) > > return -EINVAL; > > > > - if (kc->clock_get64) { > > - error = kc->clock_get64(which_clock, &kernel_tp64); > > - kernel_tp = timespec64_to_timespec(kernel_tp64); > > - } else { > > - error = kc->clock_get(which_clock, &kernel_tp); > > - } > > + error = kc->clock_get64(which_clock, &kernel_tp64); > > + if (!error) > > + return error; > > Wrong test, should be: if (error) ... > > > + > > + kernel_tp = timespec64_to_timespec(kernel_tp64); > > > > - if (!error && copy_to_user(tp, &kernel_tp, sizeof (kernel_tp))) > > The (!error && ...) was correct here! > > > + if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp))) > > error = -EFAULT; > > > > return error; > > You can simplify this like so: > > return copy_to_user(tp, &kernel_tp, sizeof(kernel_tp)) ? -EFAULT : > 0; > > > @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t, > which_clock, > > if (!kc) > > return -EINVAL; > > > > - if (kc->clock_getres64) { > > - error = kc->clock_getres64(which_clock, &rtn_tp64); > > - rtn_tp = timespec64_to_timespec(rtn_tp64); > > - } else { > > - error = kc->clock_getres(which_clock, &rtn_tp); > > - } > > + error = kc->clock_getres64(which_clock, &rtn_tp64); > > + if (!error) > > + return error; > > Also wrong. > > > + > > + rtn_tp = timespec64_to_timespec(rtn_tp64); > > > > - if (!error && tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) > > + if (tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) > > error = -EFAULT; > > > > return error; > > -- > > 1.7.9.5 > > > > Thanks, > Richard > Thanks for your comments, i'll fix these mistakes in next patch series. -- Baolin.wang Best Regards --047d7bfd05fc93d64405142428f5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On 20 April 2015 at 16:42, Richard Cochran <richardcochran@gm= ail.com> wrote:
On Mon, Apr 20, 201= 5 at 01:57:39PM +0800, Baolin Wang wrote:

> @@ -911,18 +907,14 @@ retry:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
>=C2=A0 =C2=A0 =C2=A0 =C2=A0kc =3D clockid_to_kclock(timr->it_clock);=
> -=C2=A0 =C2=A0 =C2=A0if (WARN_ON_ONCE(!kc || (!kc->timer_set &&= amp; !kc->timer_set64))) {
> +=C2=A0 =C2=A0 =C2=A0if (WARN_ON_ONCE(!kc || !kc->timer_set64)) { >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EINVA= L;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (kc->timer_set6= 4) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0new_spec64 =3D itimerspec_to_itimerspec64(new_spec);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0error =3D kc->timer_set64(timr, flags, &new_spec64,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0&old_spec64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (old_setting)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0old_spec =3D itimerspec64_to_itimerspec(= old_spec64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0error =3D kc->timer_set(timr, flags, &new_spec, rtn);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new_spec64 =3D itimer= spec_to_itimerspec64(new_spec);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->time= r_set64(timr, flags, &new_spec64,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&old_spe= c64);

This statement can fit on one line.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (old_setting)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0old_spec =3D itimerspec64_to_itimerspec(old_spec64);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0unlock_timer(timr, flag);

> @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, co= nst clockid_t, which_clock,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!kc)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
> -=C2=A0 =C2=A0 =C2=A0if (kc->clock_get64) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_get64(which_clock, &kernel_tp64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0kernel_tp =3D timespe= c64_to_timespec(kernel_tp64);
> -=C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_get(which_clock, &kernel_tp);
> -=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0error =3D kc->clock_get64(which_clock, &ke= rnel_tp64);
> +=C2=A0 =C2=A0 =C2=A0if (!error)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

Wrong test, should be: if (error) ...

> +
> +=C2=A0 =C2=A0 =C2=A0kernel_tp =3D timespec64_to_timespec(kernel_tp64)= ;
>
> -=C2=A0 =C2=A0 =C2=A0if (!error && copy_to_user(tp, &kerne= l_tp, sizeof (kernel_tp)))

The (!error && ...) was correct here!

> +=C2=A0 =C2=A0 =C2=A0if (copy_to_user(tp, &kernel_tp, sizeof (kern= el_tp)))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EFAUL= T;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

You can simplify this like so:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 return copy_to_user(tp, &kernel_tp, sizeof(= kernel_tp)) ? -EFAULT : 0;

> @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t,= which_clock,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!kc)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
> -=C2=A0 =C2=A0 =C2=A0if (kc->clock_getres64) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_getres64(which_clock, &rtn_tp64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rtn_tp =3D timespec64= _to_timespec(rtn_tp64);
> -=C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_getres(which_clock, &rtn_tp);
> -=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0error =3D kc->clock_getres64(which_clock, &= ;rtn_tp64);
> +=C2=A0 =C2=A0 =C2=A0if (!error)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

Also wrong.

> +
> +=C2=A0 =C2=A0 =C2=A0rtn_tp =3D timespec64_to_timespec(rtn_tp64);
>
> -=C2=A0 =C2=A0 =C2=A0if (!error && tp && copy_to_user(= tp, &rtn_tp, sizeof (rtn_tp)))
> +=C2=A0 =C2=A0 =C2=A0if (tp && copy_to_user(tp, &rtn_tp, s= izeof (rtn_tp)))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EFAUL= T;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return error;
> --
> 1.7.9.5
>

Thanks,
Richard

=C2=A0

Thanks for your comments, i'll fix= these mistakes in next patch series.

<= br clear=3D"all">
--
Baolin.wang
Best Regards
--047d7bfd05fc93d64405142428f5-- --===============7057139860070527656== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Content-Disposition: inline X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KTGludXhwcGMt ZGV2IG1haWxpbmcgbGlzdApMaW51eHBwYy1kZXZAbGlzdHMub3psYWJzLm9yZwpodHRwczovL2xp c3RzLm96bGFicy5vcmcvbGlzdGluZm8vbGludXhwcGMtZGV2 --===============7057139860070527656==-- From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wg0-f51.google.com (mail-wg0-f51.google.com [74.125.82.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 339CE1A1E3B for ; Mon, 20 Apr 2015 19:00:19 +1000 (AEST) Received: by wgsk9 with SMTP id k9so171121805wgs.3 for ; Mon, 20 Apr 2015 02:00:15 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <20150420084229.GA4608@localhost.localdomain> References: <1429509459-17068-1-git-send-email-baolin.wang@linaro.org> <1429509459-17068-12-git-send-email-baolin.wang@linaro.org> <20150420084229.GA4608@localhost.localdomain> Date: Mon, 20 Apr 2015 17:00:15 +0800 Message-ID: Subject: Re: [PATCH 11/11] k_clock:Remove the 32bit methods with timespec type From: Baolin Wang To: Richard Cochran Content-Type: multipart/alternative; boundary=047d7bfd05fc93d64405142428f5 Cc: pang.xunlei@linaro.org, peterz@infradead.org, heiko.carstens@de.ibm.com, paulus@samba.org, cl@linux.com, heenasirwani@gmail.com, linux-arch@vger.kernel.org, linux-s390@vger.kernel.org, y2038 Mailman List , rafael.j.wysocki@intel.com, ahh@google.com, fweisbec@gmail.com, pjt@google.com, riel@redhat.com, Arnd Bergmann , schwidefsky@de.ibm.com, John Stultz , Thomas Gleixner , rth@twiddle.net, gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org, netdev@vger.kernel.org, tj@kernel.org, linux390@de.ibm.com, linuxppc-dev@lists.ozlabs.org List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --047d7bfd05fc93d64405142428f5 Content-Type: text/plain; charset=UTF-8 On 20 April 2015 at 16:42, Richard Cochran wrote: > On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote: > > > @@ -911,18 +907,14 @@ retry: > > return -EINVAL; > > > > kc = clockid_to_kclock(timr->it_clock); > > - if (WARN_ON_ONCE(!kc || (!kc->timer_set && !kc->timer_set64))) { > > + if (WARN_ON_ONCE(!kc || !kc->timer_set64)) { > > error = -EINVAL; > > } else { > > - if (kc->timer_set64) { > > - new_spec64 = itimerspec_to_itimerspec64(new_spec); > > - error = kc->timer_set64(timr, flags, &new_spec64, > > - &old_spec64); > > - if (old_setting) > > - old_spec = > itimerspec64_to_itimerspec(old_spec64); > > - } else { > > - error = kc->timer_set(timr, flags, &new_spec, rtn); > > - } > > + new_spec64 = itimerspec_to_itimerspec64(new_spec); > > + error = kc->timer_set64(timr, flags, &new_spec64, > > + &old_spec64); > > This statement can fit on one line. > > > + if (old_setting) > > + old_spec = itimerspec64_to_itimerspec(old_spec64); > > } > > > > unlock_timer(timr, flag); > > > @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, > which_clock, > > if (!kc) > > return -EINVAL; > > > > - if (kc->clock_get64) { > > - error = kc->clock_get64(which_clock, &kernel_tp64); > > - kernel_tp = timespec64_to_timespec(kernel_tp64); > > - } else { > > - error = kc->clock_get(which_clock, &kernel_tp); > > - } > > + error = kc->clock_get64(which_clock, &kernel_tp64); > > + if (!error) > > + return error; > > Wrong test, should be: if (error) ... > > > + > > + kernel_tp = timespec64_to_timespec(kernel_tp64); > > > > - if (!error && copy_to_user(tp, &kernel_tp, sizeof (kernel_tp))) > > The (!error && ...) was correct here! > > > + if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp))) > > error = -EFAULT; > > > > return error; > > You can simplify this like so: > > return copy_to_user(tp, &kernel_tp, sizeof(kernel_tp)) ? -EFAULT : > 0; > > > @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t, > which_clock, > > if (!kc) > > return -EINVAL; > > > > - if (kc->clock_getres64) { > > - error = kc->clock_getres64(which_clock, &rtn_tp64); > > - rtn_tp = timespec64_to_timespec(rtn_tp64); > > - } else { > > - error = kc->clock_getres(which_clock, &rtn_tp); > > - } > > + error = kc->clock_getres64(which_clock, &rtn_tp64); > > + if (!error) > > + return error; > > Also wrong. > > > + > > + rtn_tp = timespec64_to_timespec(rtn_tp64); > > > > - if (!error && tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) > > + if (tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp))) > > error = -EFAULT; > > > > return error; > > -- > > 1.7.9.5 > > > > Thanks, > Richard > Thanks for your comments, i'll fix these mistakes in next patch series. -- Baolin.wang Best Regards --047d7bfd05fc93d64405142428f5 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On 20 April 2015 at 16:42, Richard Cochran <richardcochran@gm= ail.com> wrote:
On Mon, Apr 20, 201= 5 at 01:57:39PM +0800, Baolin Wang wrote:

> @@ -911,18 +907,14 @@ retry:
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
>=C2=A0 =C2=A0 =C2=A0 =C2=A0kc =3D clockid_to_kclock(timr->it_clock);=
> -=C2=A0 =C2=A0 =C2=A0if (WARN_ON_ONCE(!kc || (!kc->timer_set &&= amp; !kc->timer_set64))) {
> +=C2=A0 =C2=A0 =C2=A0if (WARN_ON_ONCE(!kc || !kc->timer_set64)) { >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EINVA= L;
>=C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (kc->timer_set6= 4) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0new_spec64 =3D itimerspec_to_itimerspec64(new_spec);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0error =3D kc->timer_set64(timr, flags, &new_spec64,
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0&old_spec64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0if (old_setting)
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0old_spec =3D itimerspec64_to_itimerspec(= old_spec64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0error =3D kc->timer_set(timr, flags, &new_spec, rtn);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0new_spec64 =3D itimer= spec_to_itimerspec64(new_spec);
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->time= r_set64(timr, flags, &new_spec64,
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&old_spe= c64);

This statement can fit on one line.

> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (old_setting)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0old_spec =3D itimerspec64_to_itimerspec(old_spec64);
>=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0unlock_timer(timr, flag);

> @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, co= nst clockid_t, which_clock,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!kc)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
> -=C2=A0 =C2=A0 =C2=A0if (kc->clock_get64) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_get64(which_clock, &kernel_tp64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0kernel_tp =3D timespe= c64_to_timespec(kernel_tp64);
> -=C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_get(which_clock, &kernel_tp);
> -=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0error =3D kc->clock_get64(which_clock, &ke= rnel_tp64);
> +=C2=A0 =C2=A0 =C2=A0if (!error)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

Wrong test, should be: if (error) ...

> +
> +=C2=A0 =C2=A0 =C2=A0kernel_tp =3D timespec64_to_timespec(kernel_tp64)= ;
>
> -=C2=A0 =C2=A0 =C2=A0if (!error && copy_to_user(tp, &kerne= l_tp, sizeof (kernel_tp)))

The (!error && ...) was correct here!

> +=C2=A0 =C2=A0 =C2=A0if (copy_to_user(tp, &kernel_tp, sizeof (kern= el_tp)))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EFAUL= T;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

You can simplify this like so:

=C2=A0 =C2=A0 =C2=A0 =C2=A0 return copy_to_user(tp, &kernel_tp, sizeof(= kernel_tp)) ? -EFAULT : 0;

> @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t,= which_clock,
>=C2=A0 =C2=A0 =C2=A0 =C2=A0if (!kc)
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL;<= br> >
> -=C2=A0 =C2=A0 =C2=A0if (kc->clock_getres64) {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_getres64(which_clock, &rtn_tp64);
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0rtn_tp =3D timespec64= _to_timespec(rtn_tp64);
> -=C2=A0 =C2=A0 =C2=A0} else {
> -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D kc->cloc= k_getres(which_clock, &rtn_tp);
> -=C2=A0 =C2=A0 =C2=A0}
> +=C2=A0 =C2=A0 =C2=A0error =3D kc->clock_getres64(which_clock, &= ;rtn_tp64);
> +=C2=A0 =C2=A0 =C2=A0if (!error)
> +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return error;

Also wrong.

> +
> +=C2=A0 =C2=A0 =C2=A0rtn_tp =3D timespec64_to_timespec(rtn_tp64);
>
> -=C2=A0 =C2=A0 =C2=A0if (!error && tp && copy_to_user(= tp, &rtn_tp, sizeof (rtn_tp)))
> +=C2=A0 =C2=A0 =C2=A0if (tp && copy_to_user(tp, &rtn_tp, s= izeof (rtn_tp)))
>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0error =3D -EFAUL= T;
>
>=C2=A0 =C2=A0 =C2=A0 =C2=A0return error;
> --
> 1.7.9.5
>

Thanks,
Richard

=C2=A0

Thanks for your comments, i'll fix= these mistakes in next patch series.

<= br clear=3D"all">
--
Baolin.wang
Best Regards
--047d7bfd05fc93d64405142428f5--