On 18 January 2018 at 22:54, Greg V <greg@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
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
