According to Jiri Denemark on 3/2/2010 8:26 AM:
If fallocate() is present, use it directly instead of posix_allocate(). If it is not support by the kernel or filesystem, emulate it using mmap() or write().
This change is to work around slow fallocate emulation done by glibc's posix_allocate() when used on files opened with O_DSYNC.
I'm personally undecided between the two approaches at the moment, but here's some stylistic review in the meantime.
dnl Availability of various common functions (non-fatal if missing). -AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap]) +AC_CHECK_FUNCS([cfmakeraw regexec uname sched_getaffinity getuid getgid posix_fallocate mmap fallocate])
This is already a long line, and you made it longer. Autoconf will let you line-wrap the list to AC_CHECK_FUNCS; it splits on whitespace, including newlines. And maybe it makes more sense to use AC_CHECK_FUNCS_ONCE instead of AC_CHECK_FUNCS (oh right, 2.59 didn't have the former).
-#ifdef HAVE_POSIX_FALLOCATE +#if defined(HAVE_POSIX_FALLOCATE) && !defined(HAVE_FALLOCATE)
Redundant parenthesis. I know upstream gnulib's maint.mk warns about them; did we turn them off in libvirt's 'make syntax-check'?
#ifdef HAVE_MMAP -int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len) +static int safezero_mmap(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len)
Already there before your patch, but formatting this as: static int safezero_mmap(int... makes life easier for emacs, and for 'grep "^safezero"' to quickly find the implementation.
@@ -196,6 +196,24 @@ int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len) return 0; } #endif /* HAVE_MMAP */ + +int safezero(int fd, int flags ATTRIBUTE_UNUSED, off_t offset, off_t len)
Likewise, for: int safezero(int...
+{ +#ifdef HAVE_FALLOCATE + if (fallocate(fd, 0, offset, len) < 0) { + if (errno != ENOSYS && errno != EOPNOTSUPP) + return -1; + } else + return 0; +#endif + +#ifdef HAVE_MMAP + return safezero_mmap(fd, 0, offset, len); +#else + return safezero_write(fd, 0, offset, len); +#endif
Looks reasonable; but why not name both fallbacks safezero_fallback at the points where they are declared (HAVE_MMAP/!HAVE_MMAP), so that you can get rid of the in-function #ifdef HAVE_MMAP here? -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org