Skip to content

Commit 950eb73

Browse files
lzsaveradamdmoss
andcommitted
chksum: fix typos, add comments, try to fix the issue
ZFS-CI-Type: linux Signed-off-by: Alexx Saver <[email protected]> Co-authored-by: Adam Moss <[email protected]>
1 parent a11cce1 commit 950eb73

File tree

1 file changed

+28
-12
lines changed

1 file changed

+28
-12
lines changed

module/zfs/zfs_chksum.c

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ typedef struct {
4949
zio_checksum_tmpl_free_t *(free);
5050
} chksum_stat_t;
5151

52+
/* a state machine for the benchmark: AT_STARTUP -> AT_BENCHMARK -> AT_DONE
53+
most likely, a blocking mechanism is needed in case of multiple requests */
54+
5255
#define AT_STARTUP 0
5356
#define AT_BENCHMARK 1
5457
#define AT_DONE 2
@@ -155,11 +158,11 @@ chksum_run(chksum_stat_t *cs, abd_t *abd, void *ctx, int round,
155158
switch (round) {
156159
case 1: /* 1k */
157160
size = 1<<10; loops = 128; break;
158-
case 2: /* 2k */
161+
case 2: /* 4k */
159162
size = 1<<12; loops = 64; break;
160-
case 3: /* 4k */
163+
case 3: /* 16k */
161164
size = 1<<14; loops = 32; break;
162-
case 4: /* 16k */
165+
case 4: /* 64k */
163166
size = 1<<16; loops = 16; break;
164167
case 5: /* 256k */
165168
size = 1<<18; loops = 8; break;
@@ -181,6 +184,7 @@ chksum_run(chksum_stat_t *cs, abd_t *abd, void *ctx, int round,
181184
} while (run_time_ns < MSEC2NSEC(1));
182185
kpreempt_enable();
183186

187+
/* will 64 bits always be enough to store the value? */
184188
run_bw = size * run_count * NANOSEC;
185189
run_bw /= run_time_ns; /* B/s */
186190
*result = run_bw/1024/1024; /* MiB/s */
@@ -212,7 +216,9 @@ chksum_benchit(chksum_stat_t *cs)
212216
chksum_run(cs, abd, ctx, 2, &cs->bs4k);
213217
chksum_run(cs, abd, ctx, 3, &cs->bs16k);
214218
chksum_run(cs, abd, ctx, 4, &cs->bs64k);
219+
/* let us try it without fixing a cosmetic issue
215220
chksum_run(cs, abd, ctx, 5, &cs->bs256k);
221+
*/
216222
chksum_run(cs, abd, ctx, 6, &cs->bs1m);
217223
abd_free(abd);
218224

@@ -250,15 +256,25 @@ chksum_benchmark(void)
250256
if (chksum_stat_limit == AT_DONE)
251257
return;
252258

253-
254-
/* count implementations */
255-
chksum_stat_cnt = 1; /* edonr */
256-
chksum_stat_cnt += 1; /* skein */
257-
chksum_stat_cnt += sha256->getcnt();
258-
chksum_stat_cnt += sha512->getcnt();
259-
chksum_stat_cnt += blake3->getcnt();
260-
chksum_stat_data = kmem_zalloc(
261-
sizeof (chksum_stat_t) * chksum_stat_cnt, KM_SLEEP);
259+
/*
260+
* Allocate the statistics array only once.
261+
*
262+
* The first call (AT_STARTUP) fills the 256 KiB benchmark for every
263+
* implementation. The second call (AT_BENCHMARK) fills the remaining
264+
* sizes. If we allocate a new array on the second call we would lose
265+
* the 256K results that were collected earlier – which is exactly why
266+
* the column appears as zero.
267+
*/
268+
if (chksum_stat_data == NULL) {
269+
/* count implementations */
270+
chksum_stat_cnt = 1; /* edonr */
271+
chksum_stat_cnt += 1; /* skein */
272+
chksum_stat_cnt += sha256->getcnt();
273+
chksum_stat_cnt += sha512->getcnt();
274+
chksum_stat_cnt += blake3->getcnt();
275+
chksum_stat_data = kmem_zalloc(
276+
sizeof (chksum_stat_t) * chksum_stat_cnt, KM_SLEEP);
277+
}
262278

263279
/* edonr - needs to be the first one here (slow CPU check) */
264280
cs = &chksum_stat_data[cbid++];

0 commit comments

Comments
 (0)