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

[Emil Velikov]

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?


> +#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.


> +#ifndef __FreeBSD__
mkstemp does exist on FreeBSD, right? If so you can drop this guard
and simplify the one in os_create_anonymous_file().


> +#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.


> +   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.

Thanks
Emil