On 01/19/2018 20:38, Emil Velikov wrote:
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?
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.
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
