Am 21.05.2018 um 01:19 schrieb Timothy Arceri: > > > On 20/05/18 22:16, Benedikt Schemmer wrote: >> There is exactly one flock in mesa and it caused mesa not to build >> on windows when shader cache was enabled. >> >> It should be possible to revert 9f8dc3bf03ec825bae7041858dda6ca2e9a34363 >> "utils: build sha1/disk cache only with Android/Autoconf" currently >> guarding the offending code with ENABLE_SHADER_CACHE >> >> This would allow shader cache to work on windows I think. > > NAK. rand() is not thread safe. > > Why are you trying to make this build on windows?
Appveyor https://ci.appveyor.com/project/mesa3d/mesa/build/3186 was the reason why large parts are guarded by ENABLE_SHADER_CACHE which prevents me from using a simple function in the patch I mentioned I can of course circumvent that by just duplicating the code I need. It just seems cleaner to have no system dependent code here. But you're right: its quite convoluted and every time I think I got it something else pops up ;) There are also other dependencies (dlfcn.h) that need to be removed. I already have, at least to some degree. I dont know how to eliminate the runtime check for the llvm build though, but that is only used by amd code so I moved it to ac_llvm_util.h for the moment which seems to work. si_pipe.c somewhere around line 800 now looks like: --- if (ac_get_function_timestamp(LLVMInitializeAMDGPUTargetInfo, &llvm_timestamp)) { res = asprintf(×tamp_str, "%s_%u", __TIMESTAMP__, llvm_timestamp); } --- >There is no use case for this on windows currently so it won't even be tested. >There are also other calls such as getuid() etc that >will fail on windows. > > If someone want this to work on windows they should just write windows > specific paths IMO. > >> >> I dont have a test system with windows though. >> This builds on linux and is tested with Deus Ex:MD and Metro 2033 Redux >> both with cold shader cache. >> >> Really >> Fixes: d1efa09d342bff3e5def2978a0bef748d74f9c82 >> >> CC: Tapani Pälli <tapani.pa...@intel.com> >> CC: "Marek Olšák" <mar...@gmail.com> >> CC: Emil Velikov <emil.l.veli...@gmail.com> >> CC: Timothy Arceri <tarc...@itsqueeze.com> >> CC: Samuel Pitoiset <samuel.pitoi...@gmail.com> >> --- >> This enables the patch >> [PATCH 1/3] mesa/main/shaderapi: Use generate_sha1() unconditionally >> >> src/util/disk_cache.c | 48 +++++++++++++++++++++++++++++++++++------------- >> 1 file changed, 35 insertions(+), 13 deletions(-) >> >> diff --git a/src/util/disk_cache.c b/src/util/disk_cache.c >> index 4a762eff20..ca47bb15fb 100644 >> --- a/src/util/disk_cache.c >> +++ b/src/util/disk_cache.c >> @@ -28,7 +28,6 @@ >> #include <string.h> >> #include <stdlib.h> >> #include <stdio.h> >> -#include <sys/file.h> >> #include <sys/types.h> >> #include <sys/stat.h> >> #include <sys/mman.h> >> @@ -848,6 +847,29 @@ struct cache_entry_file_data { >> uint32_t uncompressed_size; >> }; >> +static char * >> +generate_random_string(int length) { >> + static const char a[] = "0123456789abcdef"; >> + >> + if (length > 16) >> + return NULL; >> + >> + char buf[16]; >> + char *rndstr; >> + >> + for (int i = 0; i < length - 1; ++i) { >> + // assign a random element from the lookup table >> + buf[i] = a[rand() % (sizeof(a) - 1)]; >> + } >> + >> + buf[length - 1] = 0; >> + >> + if (asprintf(&rndstr, "%s", buf) == -1) >> + return NULL; >> + >> + return rndstr; >> +} >> + >> static void >> cache_put(void *job, int thread_index) >> { >> @@ -855,7 +877,7 @@ cache_put(void *job, int thread_index) >> int fd = -1, fd_final = -1, err, ret; >> unsigned i = 0; >> - char *filename = NULL, *filename_tmp = NULL; >> + char *filename = NULL, *filename_tmp = NULL, *random = NULL; >> struct disk_cache_put_job *dc_job = (struct disk_cache_put_job *) job; >> filename = get_cache_file(dc_job->cache, dc_job->key); >> @@ -873,7 +895,16 @@ cache_put(void *job, int thread_index) >> * final destination filename, (to prevent any readers from seeing >> * a partially written file). >> */ >> - if (asprintf(&filename_tmp, "%s.tmp", filename) == -1) >> + >> + /* This next part used to be an flock(), which would prevent windows >> systems >> + * to build. 4 hex characters should be enough to prevent filename race >> + * conditions for now. >> + */ >> + random = generate_random_string(4); >> + if (random == NULL) >> + goto done; >> + >> + if (asprintf(&filename_tmp, "%s_%s.tmp", filename, random) == -1) >> goto done; >> fd = open(filename_tmp, O_WRONLY | O_CLOEXEC | O_CREAT, 0644); >> @@ -890,16 +921,7 @@ cache_put(void *job, int thread_index) >> goto done; >> } >> - /* With the temporary file open, we take an exclusive flock on >> - * it. If the flock fails, then another process still has the file >> - * open with the flock held. So just let that file be responsible >> - * for writing the file. >> - */ >> - err = flock(fd, LOCK_EX | LOCK_NB); >> - if (err == -1) >> - goto done; >> - >> - /* Now that we have the lock on the open temporary file, we can >> + /* Now that we have the open temporary file, we can >> * check to see if the destination file already exists. If so, >> * another process won the race between when we saw that the file >> * didn't exist and now. In this case, we don't do anything more, >> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
