[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