On Oct 30, 2013, at 9:25 AM, Jeff King <p...@peff.net> wrote:

> On Sat, Oct 26, 2013 at 09:55:36AM +0200, Thomas Rast wrote:
> 
>>> The POSIX standard doesn't currently define a `nothll`/`htonll`
>> 
>> typo: ntohll
> 
> Thanks.
> 
>>> function pair to perform network-to-host and host-to-network
>>> swaps of 64-bit data. These 64-bit swaps are necessary for the on-disk
>>> storage of EWAH bitmaps if they are not in native byte order.
>> [...]
>>> +#          include <byteswap.h>
>> 
>> Do we need a hack on top similar to what ntoh_l and hton_l do, for
>> platforms that do not support unaligned access?
> 
> Ugh, probably. I didn't even know about those. But we do use them when
> reading the ewah bitmaps, which I believe can be at random offsets. We
> should be able to use the same ntoh_l solution.
> 
> -Peff

You are right, when reading mmaps we cannot ensure the alignment of the 8 byte 
sections. For writing the bitmaps to memory, however, we can make that 
assumption because the writes will happen to either malloc'ed segments, or the 
stack.

A proposed fix follows:

>From 0eed934805543390195f8aee231f8a1da33dad0b Mon Sep 17 00:00:00 2001
From: Vicent Marti <tan...@gmail.com>
Date: Wed, 30 Oct 2013 15:27:20 +0100
Subject: [PATCH] ewah: support non-aligned reads

When reading straight from a mmaped file, some platforms do not support
atomically loading 8 bytes from random positions on memory to perform a
byte swap. To work around this, we `memcpy` the whole area that needs
to be byteswapped to its final destination, and then perform the
byteswap on each 8-byte chunk: the destionation chunks *will* be aligned
because the destination area is a malloc'ed chunk of memory.

Signed-off-by: Vicent Marti <tan...@gmail.com>
---
 ewah/ewah_io.c | 24 ++++++++++++++++++------
 1 file changed, 18 insertions(+), 6 deletions(-)

diff --git a/ewah/ewah_io.c b/ewah/ewah_io.c
index fca93f9..41f6d3d 100644
--- a/ewah/ewah_io.c
+++ b/ewah/ewah_io.c
@@ -76,6 +76,10 @@ int ewah_serialize_to(struct ewah_bitmap *self,
        buffer = self->buffer;
        words_left = self->buffer_size;
 
+       /* buffer is a malloc'ed buffer, which we can assume to
+        * be at the very least 8-byte aligned */
+       assert((intptr_t)buffer % 8 == 0);
+
        while (words_left >= words_per_dump) {
                for (i = 0; i < words_per_dump; ++i, ++buffer)
                        dump[i] = htonll(*buffer);
@@ -117,7 +121,6 @@ int ewah_serialize(struct ewah_bitmap *self, int fd)
 int ewah_read_mmap(struct ewah_bitmap *self, void *map, size_t len)
 {
        uint32_t *read32 = map;
-       eword_t *read64;
        size_t i;
 
        self->bit_size = ntohl(*read32++);
@@ -128,10 +131,19 @@ int ewah_read_mmap(struct ewah_bitmap *self, void *map, 
size_t len)
        if (!self->buffer)
                return -1;
 
-       for (i = 0, read64 = (void *)read32; i < self->buffer_size; ++i)
-               self->buffer[i] = ntohll(*read64++);
+#ifdef NEEDS_ALIGNED_ACCESS
+       memcpy(self->buffer, read32, self->buffer_size * sizeof(eword_t));
+       for (i = 0; i < self->buffer_size; ++i)
+               self->buffer[i] = ntohll(self->buffer[i]);
+#else
+       {
+               eword_t *read64 = (void *)read32;
+               for (i = 0; i < self->buffer_size; ++i)
+                       self->buffer[i] = ntohll(read64[i]);
+       }
+#endif
 
-       read32 = (void *)read64;
+       read32 += self->buffer_size * sizeof(eword_t) / sizeof(uint32_t);
        self->rlw = self->buffer + ntohl(*read32++);
 
        return (3 * 4) + (self->buffer_size * 8);
@@ -175,7 +187,7 @@ int ewah_deserialize(struct ewah_bitmap *self, int fd)
                        return -1;
 
                for (i = 0; i < words_per_dump; ++i, ++buffer)
-                       *buffer = ntohll(dump[i]);
+                       *buffer = ntohll(dump[i]); /* aligned on stack */
 
                words_left -= words_per_dump;
        }
@@ -185,7 +197,7 @@ int ewah_deserialize(struct ewah_bitmap *self, int fd)
                        return -1;
 
                for (i = 0; i < words_left; ++i, ++buffer)
-                       *buffer = ntohll(dump[i]);
+                       *buffer = ntohll(dump[i]); /* aligned on stack */
        }
 
        /** 32 bit -- position for the RLW */
-- 
1.8.3.2


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to