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@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list