[BUG] etnaviv: broken timeouts

Lucas Stach l.stach at pengutronix.de
Fri Dec 1 10:58:21 UTC 2017


Am Montag, den 06.11.2017, 18:05 +0100 schrieb Arnd Bergmann:
[...]
> I would still argue that the code you posted is safe on Linux because
> we won't arbitrarily change the time base for CLOCK_MONOTONIC
> to anything that can wrap around in practice, based purely on
> the idea that doing so would break not just etnaviv but all kinds of
> other code.
> 
> In the code you have
> 
> foo(struct drm_etnaviv_timespec *ts, unsigned timeout)
> {
>          struct timespec now;
> 
>          clock_gettime(CLOCK_MONOTONIC, &now);
> 
>          ts->tv_sec = now.tv_sec + timeout / 1000;
>          ts->tv_nsec = now.tv_nsec + (timeout % 1000) * 1000 * 1000;
>          if (ts->tv_nsec > 1000 * 1000 * 1000) {
>                  ts->tv_nsec -= 1000 * 1000 * 1000;
>                  ts->tv_sec += 1;
>          }
>  }
> 
> we add at most 49 days worth of milliseconds, so that's good
> based on my assumption above that CLOCK_MONOTONIC
> can't overflow before 2038 (or 2085 if you assume it starts
> at boot time).
> 
> Even if we were to add arbitrary 32-bit seconds (signed
> or unsigned), it can still still work fine as long as we don't
> degrade to 'timespec' in the kernel ;-)

So it seems the conclusion of this discussion is that we want to go
with the initial timespec64 version of this patch + document in the
UAPI header that timeouts have to be specified in terms of
CLOCK_MONOTONIC with the assumption that this clock doesn't start out
at negative values, as is the case on Linux.

Russell, can you get me a patch for this? All the currently proposed
changes are missing a sign-off, so I can't take them.

Regards,
Lucas


More information about the etnaviv mailing list