[Mesa-dev] [PATCH] util/disk_cache: don't require 64bit atomic operations

[Matt Turner]

On Sun, Mar 5, 2017 at 11:14 AM, Grazvydas Ignotas <notasas at gmail.com> wrote:
> There are still some distributions trying to support unfortunate people
> with old or exotic CPUs that don't have 64bit atomic operations. Missing
> 64bit atomic ops can be detected at compile time, so provide a 32bit
> fallback and add an appropriate cache size limit ceiling for that case.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93089
> Signed-off-by: Grazvydas Ignotas <notasas at gmail.com>

Again, thanks for working on this.

As I alluded in the other thread, I realize that the typeless
p_atomics API glosses over a real problem -- that atomic operations
are no where close to uniformly supported across platforms. libdrm for
instance, provides an atomic_t type, which is a struct with a single
int. As far as I can tell, that's really all you can portably do
atomic operations on.

So I'd like to work towards getting rid of the typeless nature of
p_atomic*. Until that's fixed, there's not really any way to keep out
code that's going to break on some platform.

With that in mind...

> ---
> no commit access
>
>  src/util/disk_cache.c | 17 ++++++++++++++++-
>  1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c
> index 4a8ee21..daa5c7b 100644
> --- a/src/util/disk_cache.c
> +++ b/src/util/disk_cache.c
> @@ -37,6 +37,7 @@
>  #include <pwd.h>
>  #include <errno.h>
>  #include <dirent.h>
> +#include <limits.h>
>  #include "zlib.h"
>
>  #include "util/crc32.h"
> @@ -56,6 +57,11 @@
>  /* The number of keys that can be stored in the index. */
>  #define CACHE_INDEX_MAX_KEYS (1 << CACHE_INDEX_KEY_BITS)
>
> +/* limit maximum cache size on platforms that lack atomic 64bit operations */
> +#ifndef __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8
> +#define CACHE_SIZE_32BIT_LIMITED
> +#endif
> +
>  struct disk_cache {
>     /* The path to the cache directory. */
>     char *path;
> @@ -65,7 +71,11 @@ struct disk_cache {
>     size_t index_mmap_size;
>
>     /* Pointer to total size of all objects in cache (within index_mmap) */
> +#ifndef CACHE_SIZE_32BIT_LIMITED
>     uint64_t *size;
> +#else
> +   uint32_t *size;
> +#endif

I talked with Tim on IRC briefly about this. Maybe I'm just showing my
ignorance, but I'm surprised that we would need more than 4GB address
space for the shader cache.

Tim said that he thought we could reduce the size once compression
support has landed (which it now has as of commit 85a9b1b56).

Tim, can we side step this problem completely by conditionally
reducing the size to uint32_t?