Hi Russell,
Am Freitag, den 09.03.2018, 11:44 +0000 schrieb Russell King - ARM Linux:
> Hi Lucas,
>
> Please retain my authorship of my patch, which was sent on 23 Oct 2017.
> The patch you have below is 100% identical to that which I sent.
I'll gladly do so if you provide me a proper Signed-off-by for this
patch, which was missing from your 23 Oct 2017 submission.
> You should also point out, as per the follow-on discussion, that using
> clock_gettime() on 32-bit systems will not work once the time it
> reports wraps - so something like the comment I suggested in a follow
> up patch:
>
> + * Etnaviv timeouts are specified wrt CLOCK_MONOTONIC, not jiffies.
> + * We need to calculate the timeout in terms of number of jiffies
> + * between the specified timeout and the current CLOCK_MONOTONIC time.
> + * Note: clock_gettime() is 32-bit on 32-bit arch. Using 64-bit
> + * timespec math here just means that when a wrap occurs, the
> + * specified timeout goes into the past and we can't request a
> + * timeout in the future: IOW, the code breaks.
>
> would be sensible either in the commit message or the code.
I'll add it to the commit message, as I think the discussion with Arnd
established that this is a very theoretical risk, not likely to be hit
in practice.
Regards,
Lucas
> On Fri, Mar 09, 2018 at 12:21:49PM +0100, Lucas Stach wrote:
> > The old way did clamp the jiffy conversion and thus caused the timeouts
> > to become negative after some time. Also it didn't work with userspace
> > which actually fills the upper 32bits of the 64bit timestamp value.
> >
> > Fix this by using the solution developed and tested by Russell.
> >
> > > > Suggested-by: Russell King <linux@xxxxxxxxxxxxxxx>
> > > > Signed-off-by: Lucas Stach <l.stach@xxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/etnaviv/etnaviv_drv.h | 25 +++++++++++++++++--------
> > 1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.h b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > index ddb17ee565e9..17a43da98fb9 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.h
> > @@ -26,6 +26,7 @@
> > #include <linux/pm_runtime.h>
> > #include <linux/slab.h>
> > #include <linux/list.h>
> > +#include <linux/time64.h>
> > #include <linux/types.h>
> > #include <linux/sizes.h>
> >
> > @@ -132,19 +133,27 @@ static inline bool fence_after_eq(u32 a, u32 b)
> > > > return (s32)(a - b) >= 0;
> > }
> >
> > +/*
> > + * Etnaviv timeouts are specified wrt CLOCK_MONOTONIC, not jiffies.
> > + * We need to calculate the timeout in terms of number of jiffies
> > + * between the specified timeout and the current CLOCK_MONOTONIC time.
> > + */
> > static inline unsigned long etnaviv_timeout_to_jiffies(
> > > > const struct timespec *timeout)
> > {
> > > > - unsigned long timeout_jiffies = timespec_to_jiffies(timeout);
> > > > - unsigned long start_jiffies = jiffies;
> > > > - unsigned long remaining_jiffies;
> > > > + struct timespec64 ts, to;
> > +
> > > > + to = timespec_to_timespec64(*timeout);
> > +
> > > > + ktime_get_ts64(&ts);
> > +
> > > > + /* timeouts before "now" have already expired */
> > > > + if (timespec64_compare(&to, &ts) <= 0)
> > > > + return 0;
> >
> > > > - if (time_after(start_jiffies, timeout_jiffies))
> > > > - remaining_jiffies = 0;
> > > > - else
> > > > - remaining_jiffies = timeout_jiffies - start_jiffies;
> > > > + ts = timespec64_sub(to, ts);
> >
> > > > - return remaining_jiffies;
> > > > + return timespec64_to_jiffies(&ts);
> > }
> >
> > #endif /* __ETNAVIV_DRV_H__ */
> > --
> > 2.16.1
> >
>
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
