[Mesa-dev] [PATCH v2 3/5] util: add anon_file.h for all memfd/temp file usage

[Greg V]

On 01/19/2018 20:38, Emil Velikov wrote:
> On 18 January 2018 at 22:54, Greg V <greg at unrelenting.technology> wrote:
>> Move the Weston os_create_anonymous_file code from egl/wayland into util,
>> add support for Linux memfd and FreeBSD SHM_ANON,
>> use that code in anv instead of explicit memfd calls for portability.
>> ---
> Looks quite good - a few small suggestions below:
>
>
>> -   pool->fd = memfd_create("block pool", MFD_CLOEXEC);
>> +   pool->fd = os_create_anonymous_file(BLOCK_POOL_MEMFD_SIZE);
> Don't know how much the name matters, Jason?
I can add it back. I guess it's slightly nicer for debugging.
>> +#elif HAVE_LINUX_MEMFD_H
> Swap the AC_CHEKC_FUNC memfd_create checks in configure.ac/meson.build
> with check AC_CHECK_HEADER linux/memfd.h ones.
Okay.
>> +#ifndef __FreeBSD__
> mkstemp does exist on FreeBSD, right? If so you can drop this guard
> and simplify the one in os_create_anonymous_file().
The guard is there because this function (create_tmpfile_cloexec) goes 
unused on FreeBSD. os_create_anonymous_file uses shm_open, which does 
not take a filename.
I could've used shm_open inside create_tmpfile_cloexec but that would 
cause compiler warnings about unused arguments…

I guess I'll just merge create_tmpfile_cloexec into 
os_create_anonymous_file, it's currently separated for no good reason.
>> +#if defined(HAVE_POSIX_FALLOCATE) && !defined(__FreeBSD__)
>> +   do {
>> +      ret = posix_fallocate(fd, 0, size);
>> +   } while (ret == EINTR);
>> +   if (ret != 0) {
>> +      close(fd);
>> +      errno = ret;
>> +      return -1;
>> +   }
>> +#else
> I'd keep this hunk as follow-up. The SIGBUS comment sounds scary, plus
> there's no AC_CHECK_FUNC or equivalent across the build systems.
Oh, sorry. I just copy-pasted the whole function from the current Weston 
code (since I already had memfd/shm stuff in my fork of Weston).
>> +   do {
>> +      ret = ftruncate(fd, size);
>> +   } while (ret < 0 && errno == EINTR);
> Either add a comment (+ tiny mentioned in commit summary) about the
> while/EINTR or keep as follow-up.
> This does add extra robustness, yet code it's not tested so often so
> I'm worried about side-effects going undetected.
Same. Yeah I can remove this.