Skip to content

Commit b581b2e

Browse files
jamieprydejamiepryde
authored
Merge pull request ceph#62657 from jamiepryde/ec-plugins-tidying-nits-and-bits
erasure-code: Handle review comments from ceph#61804 to tidy up EC plugin changes
2 parents 4e32f26 + 87ff016 commit b581b2e

File tree

9 files changed

+240
-164
lines changed

9 files changed

+240
-164
lines changed

‎src/erasure-code/ErasureCode.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ int ErasureCode::sanity_check_k_m(int k, int m, ostream *ss)
111111
*ss << "m=" << m << " must be >= 1" << std::endl;
112112
return -EINVAL;
113113
}
114-
int max_k_plus_m = std::numeric_limits<decltype(shard_id_t::id)>::max();
114+
auto max_k_plus_m = std::numeric_limits<decltype(shard_id_t::id)>::max();
115115
if (k+m > max_k_plus_m) {
116116
*ss << "(k+m)=" << (k+m) << " must be <= " << max_k_plus_m << std::endl;
117117
return -EINVAL;
@@ -136,7 +136,7 @@ int ErasureCode::_minimum_to_decode(const set<int> &want_to_read,
136136
set<int> *minimum)
137137
{
138138
if (includes(available_chunks.begin(), available_chunks.end(),
139-
want_to_read.begin(), want_to_read.end())) {
139+
want_to_read.begin(), want_to_read.end())) {
140140
*minimum = want_to_read;
141141
} else {
142142
unsigned int k = get_data_chunk_count();
@@ -447,7 +447,7 @@ int ErasureCode::_decode(const shard_id_set &want_to_read,
447447
(*decoded)[i].rebuild_aligned(SIMD_ALIGN);
448448
}
449449
bufferlist &bl = (*decoded)[i];
450-
if (bl.length() != bl.begin().get_current_ptr().length()) {
450+
if (!bl.is_contiguous()) {
451451
bl.rebuild();
452452
}
453453
}

‎src/erasure-code/ErasureCodeInterface.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,13 @@ namespace ceph {
309309
virtual int minimum_to_decode(const shard_id_set &want_to_read,
310310
const shard_id_set &available,
311311
shard_id_set &minimum_set,
312-
mini_flat_map<shard_id_t, std::vector<std::pair<int, int>>>
313-
*minimum_sub_chunks) = 0;
312+
mini_flat_map<shard_id_t, std::vector<std::pair<int, int>>> *minimum_sub_chunks) = 0;
314313

315314
// Interface for legacy EC.
316315
[[deprecated]]
317316
virtual int minimum_to_decode(const std::set<int> &want_to_read,
318317
const std::set<int> &available,
319-
std::map<int, std::vector<std::pair<int, int>>>
320-
*minimum) = 0;
318+
std::map<int, std::vector<std::pair<int, int>>> *minimum) = 0;
321319

322320
/**
323321
* Compute the smallest subset of **available** chunks that needs

‎src/erasure-code/isa/ErasureCodeIsa.cc‎

Lines changed: 78 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -104,10 +104,11 @@ int ErasureCodeIsa::decode_chunks(const set<int> &want_to_read,
104104
erasures[erasures_count] = i;
105105
erasures_count++;
106106
}
107-
if (i < k)
107+
if (i < k) {
108108
data[i] = (*decoded)[i].c_str();
109-
else
109+
} else {
110110
coding[i - k] = (*decoded)[i].c_str();
111+
}
111112
}
112113
erasures[erasures_count] = -1;
113114
ceph_assert(erasures_count > 0);
@@ -122,21 +123,29 @@ int ErasureCodeIsa::encode_chunks(const shard_id_map<bufferptr> &in,
122123
uint64_t size = 0;
123124

124125
for (auto &&[shard, ptr] : in) {
125-
if (size == 0) size = ptr.length();
126-
else ceph_assert(size == ptr.length());
126+
if (size == 0) {
127+
size = ptr.length();
128+
} else {
129+
ceph_assert(size == ptr.length());
130+
}
127131
chunks[static_cast<int>(shard)] = const_cast<char*>(ptr.c_str());
128132
}
129133

130134
for (auto &&[shard, ptr] : out) {
131-
if (size == 0) size = ptr.length();
132-
else ceph_assert(size == ptr.length());
135+
if (size == 0) {
136+
size = ptr.length();
137+
} else {
138+
ceph_assert(size == ptr.length());
139+
}
133140
chunks[static_cast<int>(shard)] = ptr.c_str();
134141
}
135142

136143
char *zeros = nullptr;
137144

138145
for (shard_id_t i; i < k + m; ++i) {
139-
if (in.contains(i) || out.contains(i)) continue;
146+
if (in.contains(i) || out.contains(i)) {
147+
continue;
148+
}
140149

141150
if (zeros == nullptr) {
142151
zeros = (char*)malloc(size);
@@ -148,14 +157,16 @@ int ErasureCodeIsa::encode_chunks(const shard_id_map<bufferptr> &in,
148157

149158
isa_encode(&chunks[0], &chunks[k], size);
150159

151-
if (zeros != nullptr) free(zeros);
160+
if (zeros != nullptr) {
161+
free(zeros);
162+
}
152163

153164
return 0;
154165
}
155166

156167
int ErasureCodeIsa::decode_chunks(const shard_id_set &want_to_read,
157168
shard_id_map<bufferptr> &in,
158-
shard_id_map<bufferptr> &out)
169+
shard_id_map<bufferptr> &out)
159170
{
160171
unsigned int size = 0;
161172
shard_id_set erasures_set;
@@ -169,33 +180,48 @@ int ErasureCodeIsa::decode_chunks(const shard_id_set &want_to_read,
169180
memset(coding, 0, sizeof(char*) * m);
170181

171182
for (auto &&[shard, ptr] : in) {
172-
if (size == 0) size = ptr.length();
173-
else ceph_assert(size == ptr.length());
174-
if (shard < k) {
175-
data[static_cast<int>(shard)] = const_cast<char*>(ptr.c_str());
183+
if (size == 0) {
184+
size = ptr.length();
185+
} else {
186+
ceph_assert(size == ptr.length());
176187
}
177-
else {
178-
coding[static_cast<int>(shard) - k] = const_cast<char*>(ptr.c_str());
188+
189+
if (shard < k) {
190+
data[static_cast<int>(shard)] = ptr.c_str();
191+
} else {
192+
coding[static_cast<int>(shard) - k] = ptr.c_str();
179193
}
180194
erasures_set.erase(shard);
181195
}
182196

183197
for (auto &&[shard, ptr] : out) {
184-
if (size == 0) size = ptr.length();
185-
else ceph_assert(size == ptr.length());
186-
if (shard < k) {
187-
data[static_cast<int>(shard)] = const_cast<char*>(ptr.c_str());
198+
if (size == 0) {
199+
size = ptr.length();
200+
} else {
201+
ceph_assert(size == ptr.length());
188202
}
189-
else {
190-
coding[static_cast<int>(shard) - k] = const_cast<char*>(ptr.c_str());
203+
204+
if (shard < k) {
205+
data[static_cast<int>(shard)] = ptr.c_str();
206+
} else {
207+
coding[static_cast<int>(shard) - k] = ptr.c_str();
191208
}
209+
erasures_set.insert(shard);
192210
}
193211

194212
for (int i = 0; i < k + m; i++) {
195213
char **buf = i < k ? &data[i] : &coding[i - k];
196214
if (*buf == nullptr) {
197215
*buf = (char *)malloc(size);
216+
ceph_assert(buf != nullptr);
198217
to_free.insert(shard_id_t(i));
218+
/* If buffer was not provided, is not an erasure (i.e. in the out map),
219+
* and a data shard, then it can be assumed to be zero. This is most
220+
* likely due to EC shards being different sizes.
221+
*/
222+
if (i < k && !erasures_set.contains(shard_id_t(i))) {
223+
memset(*buf, 0, size);
224+
}
199225
}
200226
}
201227

@@ -222,7 +248,7 @@ void
222248
ErasureCodeIsa::isa_xor(char **data, char *coding, int blocksize, int data_vectors)
223249
{
224250
ceph_assert(data_vectors <= MAX_K);
225-
char * xor_bufs[MAX_K + 1];
251+
char *xor_bufs[MAX_K + 1];
226252
for (int i = 0; i < data_vectors; i++) {
227253
xor_bufs[i] = data[i];
228254
}
@@ -232,7 +258,10 @@ ErasureCodeIsa::isa_xor(char **data, char *coding, int blocksize, int data_vecto
232258
// Otherwise, use byte_xor()
233259
bool aligned = true;
234260
for (int i = 0; i <= data_vectors; i++) {
235-
aligned &= is_aligned(xor_bufs[i], EC_ISA_ADDRESS_ALIGNMENT);
261+
if (!is_aligned(xor_bufs[i], EC_ISA_ADDRESS_ALIGNMENT)) {
262+
aligned = false;
263+
break;
264+
}
236265
}
237266

238267
if (aligned) {
@@ -289,13 +318,13 @@ ErasureCodeIsaDefault::encode_delta(const bufferptr &old_data,
289318
const bufferptr &new_data,
290319
bufferptr *delta_maybe_in_place)
291320
{
292-
constexpr int data_vectors = 2;
293-
char * data[data_vectors];
321+
constexpr int NUM_DATA_VECTORS = 2;
322+
char * data[NUM_DATA_VECTORS];
294323
data[0] = const_cast<char*>(old_data.c_str());
295324
data[1] = const_cast<char*>(new_data.c_str());
296325
char * coding = delta_maybe_in_place->c_str();
297326

298-
isa_xor(data, coding, delta_maybe_in_place->length(), data_vectors);
327+
isa_xor(data, coding, delta_maybe_in_place->length(), NUM_DATA_VECTORS);
299328
}
300329

301330
// -----------------------------------------------------------------------------
@@ -308,23 +337,29 @@ ErasureCodeIsaDefault::apply_delta(const shard_id_map<bufferptr> &in,
308337
const unsigned blocksize = first->second.length();
309338

310339
for (auto const& [datashard, databuf] : in) {
311-
if (datashard < k) {
312-
for (auto const& [codingshard, codingbuf] : out) {
313-
if (codingshard >= k) {
314-
ceph_assert(codingbuf.length() == blocksize);
315-
if (m==1) {
316-
constexpr int data_vectors = 2;
317-
char * data[data_vectors];
318-
data[0] = const_cast<char*>(databuf.c_str());
319-
data[1] = const_cast<char*>(codingbuf.c_str());
320-
char * coding = const_cast<char*>(codingbuf.c_str());
321-
isa_xor(data, coding, blocksize, data_vectors);
322-
} else {
323-
unsigned char* data = reinterpret_cast<unsigned char*>(const_cast<char*>(databuf.c_str()));
324-
unsigned char* coding = reinterpret_cast<unsigned char*>(const_cast<char*>(codingbuf.c_str()));
325-
ec_encode_data_update(blocksize, k, 1, static_cast<int>(datashard), encode_tbls + (32 * k * (static_cast<int>(codingshard) - k)), data, &coding);
326-
}
327-
}
340+
if (datashard >= k) {
341+
continue;
342+
}
343+
for (auto const& [codingshard, codingbuf] : out) {
344+
if (codingshard < k) {
345+
continue;
346+
}
347+
ceph_assert(codingbuf.length() == blocksize);
348+
if (m == 1) {
349+
constexpr int NUM_DATA_VECTORS = 2;
350+
char * data[NUM_DATA_VECTORS];
351+
data[0] = const_cast<char*>(databuf.c_str());
352+
data[1] = codingbuf.c_str();
353+
char * coding = codingbuf.c_str();
354+
isa_xor(data, coding, blocksize, NUM_DATA_VECTORS);
355+
} else {
356+
unsigned char* data = reinterpret_cast<unsigned char*>(const_cast<char*>(databuf.c_str()));
357+
unsigned char* coding = reinterpret_cast<unsigned char*>(codingbuf.c_str());
358+
// We always update one parity at a time, so specify 1 row, and a pointer to the
359+
// correct row in encode_tbls for this particular parity
360+
ec_encode_data_update(blocksize, k, 1, static_cast<int>(datashard),
361+
encode_tbls + (32 * k * (static_cast<int>(codingshard) - k)),
362+
data, &coding);
328363
}
329364
}
330365
}

0 commit comments

Comments
 (0)