On Oct 30, 2013, at 9:25 AM, Jeff King <peff@xxxxxxxx> 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 <tanoku@xxxxxxxxx>
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 <tanoku@xxxxxxxxx>
---
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html