diff options
author | Pete Zaitcev <zaitcev@kotori.zaitcev.us> | 2017-05-23 17:45:43 -0600 |
---|---|---|
committer | Thiago da Silva <thiago@redhat.com> | 2017-06-06 14:53:45 -0400 |
commit | 960cdd09dcabe5e125d8ee1c54fcc1f908e8ef11 (patch) | |
tree | 51e7d4237bedd2129b38fa3eb2b4733e59252fb0 | |
parent | de984f59e7a6cca8c1f3cf1d93b87ff144a2b974 (diff) |
Stop using ceill() to compute padded data size
The well-known idiom to compute a required number of data blocks
of size B to contain data of length d is:
(d + (B-1))/B
The code we use, with ceill(), computes the same value, but does
it in an unorthodox way. This makes a reviewer to doubt himself
and even run tests to make sure we're really computing the
obvious thing.
Apropos the reviewer confusion, the code in Phazr.IO looks weird.
It uses (word_size - hamming_distance) to compute the necessary
number of blocks... but then returns the amount of memory needed
to store blocks of a different size (word_size). We left all of it
alone and return exactly the same values that the old computation
returned.
All these computations were the only thing in the code that used
-lm, so drop that too.
Coincidentially, this patch solves the crash of distro-built
packages of liberasurecode (see Red Hat bug #1454543). But it's
a side effect. Expect a proper patch soon.
Change-Id: Ib297f6df304abf5ca8c27d3392b1107a525e0be0
Notes
Notes (review):
Code-Review+2: Thiago da Silva <thiago@redhat.com>
Code-Review+2: Tim Burke <tim@swiftstack.com>
Workflow+1: Tim Burke <tim@swiftstack.com>
Verified+2: Jenkins
Submitted-by: Jenkins
Submitted-at: Tue, 06 Jun 2017 19:17:12 +0000
Reviewed-on: https://review.openstack.org/467418
Project: openstack/liberasurecode
Branch: refs/heads/master
-rw-r--r-- | configure.ac | 2 | ||||
-rw-r--r-- | include/erasurecode/erasurecode_stdinc.h | 3 | ||||
-rw-r--r-- | src/backends/phazrio/libphazr.c | 2 | ||||
-rw-r--r-- | src/erasurecode.c | 8 | ||||
-rw-r--r-- | src/erasurecode_helpers.c | 4 |
5 files changed, 8 insertions, 11 deletions
diff --git a/configure.ac b/configure.ac index c3509d2..d0230a0 100644 --- a/configure.ac +++ b/configure.ac | |||
@@ -88,7 +88,7 @@ dnl Check for C library headers | |||
88 | AC_HEADER_STDC | 88 | AC_HEADER_STDC |
89 | AC_CHECK_HEADERS(sys/types.h stdio.h stdlib.h stddef.h stdarg.h \ | 89 | AC_CHECK_HEADERS(sys/types.h stdio.h stdlib.h stddef.h stdarg.h \ |
90 | malloc.h memory.h string.h strings.h inttypes.h \ | 90 | malloc.h memory.h string.h strings.h inttypes.h \ |
91 | stdint.h ctype.h math.h iconv.h signal.h dlfcn.h \ | 91 | stdint.h ctype.h iconv.h signal.h dlfcn.h \ |
92 | pthread.h unistd.h limits.h errno.h syslog.h) | 92 | pthread.h unistd.h limits.h errno.h syslog.h) |
93 | AC_CHECK_FUNCS(malloc calloc realloc free openlog) | 93 | AC_CHECK_FUNCS(malloc calloc realloc free openlog) |
94 | 94 | ||
diff --git a/include/erasurecode/erasurecode_stdinc.h b/include/erasurecode/erasurecode_stdinc.h index 903b33e..aaa9e2b 100644 --- a/include/erasurecode/erasurecode_stdinc.h +++ b/include/erasurecode/erasurecode_stdinc.h | |||
@@ -100,9 +100,6 @@ | |||
100 | #ifdef HAVE_ERRNO_H | 100 | #ifdef HAVE_ERRNO_H |
101 | # include <errno.h> | 101 | # include <errno.h> |
102 | #endif | 102 | #endif |
103 | #ifdef HAVE_MATH_H | ||
104 | # include <math.h> | ||
105 | #endif | ||
106 | 103 | ||
107 | #if defined(__GNUC__) && __GNUC__ >= 4 | 104 | #if defined(__GNUC__) && __GNUC__ >= 4 |
108 | # define DECLSPEC __attribute__ ((visibility("default"))) | 105 | # define DECLSPEC __attribute__ ((visibility("default"))) |
diff --git a/src/backends/phazrio/libphazr.c b/src/backends/phazrio/libphazr.c index 74ad8ad..41ed74e 100644 --- a/src/backends/phazrio/libphazr.c +++ b/src/backends/phazrio/libphazr.c | |||
@@ -88,7 +88,7 @@ struct libphazr_descriptor { | |||
88 | static int get_padded_blocksize(int w, int hd, int blocksize) | 88 | static int get_padded_blocksize(int w, int hd, int blocksize) |
89 | { | 89 | { |
90 | int word_size = w / 8; | 90 | int word_size = w / 8; |
91 | return (int) ceill((double) blocksize / (word_size - hd)) * word_size; | 91 | return ((blocksize + ((word_size - hd) - 1)) / (word_size - hd)) * word_size; |
92 | } | 92 | } |
93 | 93 | ||
94 | static int pio_matrix_encode(void *desc, char **data, char **parity, int blocksize) | 94 | static int pio_matrix_encode(void *desc, char **data, char **parity, int blocksize) |
diff --git a/src/erasurecode.c b/src/erasurecode.c index fb6d5de..d4a06c2 100644 --- a/src/erasurecode.c +++ b/src/erasurecode.c | |||
@@ -1194,8 +1194,8 @@ int liberasurecode_verify_stripe_metadata(int desc, | |||
1194 | /** | 1194 | /** |
1195 | * This computes the aligned size of a buffer passed into | 1195 | * This computes the aligned size of a buffer passed into |
1196 | * the encode function. The encode function must pad fragments | 1196 | * the encode function. The encode function must pad fragments |
1197 | * to be algined with the word size (w) and the last fragment also | 1197 | * to be aligned with the word size (w) and the last fragment also |
1198 | * needs to be aligned. This computes the sum of the algined fragment | 1198 | * needs to be aligned. This computes the sum of the aligned fragment |
1199 | * sizes for a given buffer to encode. | 1199 | * sizes for a given buffer to encode. |
1200 | */ | 1200 | */ |
1201 | int liberasurecode_get_aligned_data_size(int desc, uint64_t data_len) | 1201 | int liberasurecode_get_aligned_data_size(int desc, uint64_t data_len) |
@@ -1218,8 +1218,8 @@ int liberasurecode_get_aligned_data_size(int desc, uint64_t data_len) | |||
1218 | 1218 | ||
1219 | alignment_multiple = k * word_size; | 1219 | alignment_multiple = k * word_size; |
1220 | 1220 | ||
1221 | ret = (int) ceill( (double) | 1221 | ret = ((data_len + alignment_multiple - 1) / alignment_multiple) |
1222 | data_len / alignment_multiple) * alignment_multiple; | 1222 | * alignment_multiple; |
1223 | 1223 | ||
1224 | out: | 1224 | out: |
1225 | return ret; | 1225 | return ret; |
diff --git a/src/erasurecode_helpers.c b/src/erasurecode_helpers.c index 40db93c..fd14298 100644 --- a/src/erasurecode_helpers.c +++ b/src/erasurecode_helpers.c | |||
@@ -199,8 +199,8 @@ int get_aligned_data_size(ec_backend_t instance, int data_len) | |||
199 | alignment_multiple = k * word_size; | 199 | alignment_multiple = k * word_size; |
200 | } | 200 | } |
201 | 201 | ||
202 | aligned_size = (int) | 202 | aligned_size = ((data_len + alignment_multiple - 1) / alignment_multiple) |
203 | ceill((double) data_len / alignment_multiple) * alignment_multiple; | 203 | * alignment_multiple; |
204 | 204 | ||
205 | return aligned_size; | 205 | return aligned_size; |
206 | } | 206 | } |