[libvirt] [PATCH] util: fix thinko in runIO
When aligning you need to clear the bits in the mask and leave the others aside. Likely this code has never run, and will never run. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/util/iohelper.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 9e7bbde..93154f8 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -98,7 +98,7 @@ runIO(const char *path, int fd, int oflags, unsigned long long length) goto cleanup; } base = buf; - buf = (char *) (((intptr_t) base + alignMask) & alignMask); + buf = (char *) (((intptr_t) base + alignMask) & ~alignMask); #endif switch (oflags & O_ACCMODE) { -- 1.7.7.1
On 11/24/2011 08:36 AM, Paolo Bonzini wrote:
When aligning you need to clear the bits in the mask and leave the others aside. Likely this code has never run, and will never run.
Indeed; Linux has posix_memalign, and mingw never runs the io helper (although it does compile it, hence the #if). If gnulib would give us posix_memalign on mingw, we could nuke this #if altogether.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- src/util/iohelper.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/util/iohelper.c b/src/util/iohelper.c index 9e7bbde..93154f8 100644 --- a/src/util/iohelper.c +++ b/src/util/iohelper.c @@ -98,7 +98,7 @@ runIO(const char *path, int fd, int oflags, unsigned long long length) goto cleanup; } base = buf; - buf = (char *) (((intptr_t) base + alignMask) & alignMask); + buf = (char *) (((intptr_t) base + alignMask) & ~alignMask);
ACK and pushed. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Indeed; Linux has posix_memalign, and mingw never runs the io helper (although it does compile it, hence the #if). If gnulib would give us posix_memalign on mingw, we could nuke this #if altogether.
That's pretty difficult (unless you also add a posix_memalign_free) because at the time posix_memalign returns you have lost the base pointer for free(). Paolo
[adding bug-gnulib; replies can drop libvirt] On 11/25/2011 05:51 AM, Paolo Bonzini wrote:
Indeed; Linux has posix_memalign, and mingw never runs the io helper (although it does compile it, hence the #if). If gnulib would give us posix_memalign on mingw, we could nuke this #if altogether.
That's pretty difficult (unless you also add a posix_memalign_free) because at the time posix_memalign returns you have lost the base pointer for free().
Providing a posix_memalign_free defeats the purpose - POSIX requires that plain free() will cover the memory returned by posix_memalign. The list of platforms missing posix_memalign is a bit daunting: MacOS X 10.5, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.5.x, mingw, MSVC 9, Interix 3.5, BeOS. but what would be interesting to know is how many of those platforms return page-aligned pointers for any malloc() request of a page or more of memory. That is, we may be able to coerce malloc into aligned results by over-allocating and over-aligning the user's request, if the system malloc() has at least one mode of returning page-aligned memory. Another alternative is to override free() at the same time as providing posix_memalign(). The gnulib malloca module provides a freea() function that can distinguish whether it is paired with an alloca() (nop) or mmalloca() (call the real free()), by over-allocating and probing for a magic header. Similar concepts could be used in writing an rpl_free() that determines whether it was allocated by posix_memalign(): if we guarantee that posix_memalign always returns something aligned to at least a page, then rpl_free() can make a quick check for whether the pointer passed in is page-aligned; if so, do a hash lookup to see if it was a page reserved by our posix_memalign (if so, we know the real address to free); if not or the hash lookup fails, it came from malloc (in which case, regular free should work). Of course, unlike freea() that probes for a magic header, we can't safely probe a header by crossing page boundaries, since that would fault if we didn't allocate the previous page at the same time. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 11/25/2011 07:38 AM, Eric Blake wrote:
[adding bug-gnulib; replies can drop libvirt]
Providing a posix_memalign_free defeats the purpose - POSIX requires that plain free() will cover the memory returned by posix_memalign. The list of platforms missing posix_memalign is a bit daunting:
MacOS X 10.5, FreeBSD 6.0, NetBSD 3.0, OpenBSD 3.8, Minix 3.1.8, AIX 5.1, HP-UX 11, IRIX 6.5, OSF/1 5.1, Solaris 10, Cygwin 1.5.x, mingw, MSVC 9, Interix 3.5, BeOS.
Many of these systems have valloc(3) which could be used for this purpose. Peter
participants (3)
-
Eric Blake -
Paolo Bonzini -
Peter O'Gorman