Skip to content

Commit

Permalink
Don't emit checksum histograms in ereport.fs.zfs.checksum events
Browse files Browse the repository at this point in the history
The checksum histograms were intended to be used with ATA and parallel
SCSI, which are obsolete.  With modern storage hardware, they will
almost always look like white noise; all bits will be wrong.  They only
serve to bloat the event.  That's a particular problem on FreeBSD, where
events must fit into a 1016 byte buffer.

This fixes issue #14717 for RAIDZ pools, but not for mirror pools.

Reviewed-by: Brian Behlendorf <[email protected]>
Reviewed-by: Rich Ercolani <[email protected]>
Signed-off-by: Alan Somers <[email protected]>
Sponsored-by: Axcient
Closes #15052
  • Loading branch information
asomers authored and behlendorf committed Jul 21, 2023
1 parent ab0b039 commit cf2a225
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 41 deletions.
2 changes: 0 additions & 2 deletions include/sys/fm/fs/zfs.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,6 @@ extern "C" {
#define FM_EREPORT_PAYLOAD_ZFS_BAD_RANGE_CLEARS "bad_range_clears"
#define FM_EREPORT_PAYLOAD_ZFS_BAD_SET_BITS "bad_set_bits"
#define FM_EREPORT_PAYLOAD_ZFS_BAD_CLEARED_BITS "bad_cleared_bits"
#define FM_EREPORT_PAYLOAD_ZFS_BAD_SET_HISTOGRAM "bad_set_histogram"
#define FM_EREPORT_PAYLOAD_ZFS_BAD_CLEARED_HISTOGRAM "bad_cleared_histogram"
#define FM_EREPORT_PAYLOAD_ZFS_SNAPSHOT_NAME "snapshot_name"
#define FM_EREPORT_PAYLOAD_ZFS_DEVICE_NAME "device_name"
#define FM_EREPORT_PAYLOAD_ZFS_RAW_DEVICE_NAME "raw_name"
Expand Down
19 changes: 1 addition & 18 deletions man/man8/zpool-events.8
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
.\" Copyright 2017 Nexenta Systems, Inc.
.\" Copyright (c) 2017 Open-E, Inc. All Rights Reserved.
.\"
.Dd May 27, 2021
.Dd July 11, 2023
.Dt ZPOOL-EVENTS 8
.Os
.
Expand Down Expand Up @@ -362,23 +362,6 @@ Like
but contains
.Pq Ar good data No & ~( Ns Ar bad data ) ;
that is, the bits set in the good data which are cleared in the bad data.
.It Sy bad_set_histogram
If this field exists, it is an array of counters.
Each entry counts bits set in a particular bit of a big-endian uint64 type.
The first entry counts bits
set in the high-order bit of the first byte, the 9th byte, etc, and the last
entry counts bits set of the low-order bit of the 8th byte, the 16th byte, etc.
This information is useful for observing a stuck bit in a parallel data path,
such as IDE or parallel SCSI.
.It Sy bad_cleared_histogram
If this field exists, it is an array of counters.
Each entry counts bit clears in a particular bit of a big-endian uint64 type.
The first entry counts bits
clears of the high-order bit of the first byte, the 9th byte, etc, and the
last entry counts clears of the low-order bit of the 8th byte, the 16th byte,
etc.
This information is useful for observing a stuck bit in a parallel data
path, such as IDE or parallel SCSI.
.El
.
.Sh I/O STAGES
Expand Down
25 changes: 4 additions & 21 deletions module/zfs/zfs_fm.c
Original file line number Diff line number Diff line change
Expand Up @@ -754,10 +754,6 @@ zfs_ereport_start(nvlist_t **ereport_out, nvlist_t **detector_out,
#define MAX_RANGES 16

typedef struct zfs_ecksum_info {
/* histograms of set and cleared bits by bit number in a 64-bit word */
uint8_t zei_histogram_set[sizeof (uint64_t) * NBBY];
uint8_t zei_histogram_cleared[sizeof (uint64_t) * NBBY];

/* inline arrays of bits set and cleared. */
uint64_t zei_bits_set[ZFM_MAX_INLINE];
uint64_t zei_bits_cleared[ZFM_MAX_INLINE];
Expand All @@ -781,18 +777,16 @@ typedef struct zfs_ecksum_info {
} zfs_ecksum_info_t;

static void
update_histogram(uint64_t value_arg, uint8_t *hist, uint32_t *count)
update_bad_bits(uint64_t value_arg, uint32_t *count)
{
size_t i;
size_t bits = 0;
uint64_t value = BE_64(value_arg);

/* We store the bits in big-endian (largest-first) order */
for (i = 0; i < 64; i++) {
if (value & (1ull << i)) {
hist[63 - i] = MAX(hist[63 - i], hist[63 - i] + 1);
if (value & (1ull << i))
++bits;
}
}
/* update the count of bits changed */
*count += bits;
Expand Down Expand Up @@ -1010,10 +1004,8 @@ annotate_ecksum(nvlist_t *ereport, zio_bad_cksum_t *info,
offset++;
}

update_histogram(set, eip->zei_histogram_set,
&eip->zei_range_sets[range]);
update_histogram(cleared, eip->zei_histogram_cleared,
&eip->zei_range_clears[range]);
update_bad_bits(set, &eip->zei_range_sets[range]);
update_bad_bits(cleared, &eip->zei_range_clears[range]);
}

/* convert to byte offsets */
Expand Down Expand Up @@ -1049,15 +1041,6 @@ annotate_ecksum(nvlist_t *ereport, zio_bad_cksum_t *info,
DATA_TYPE_UINT8_ARRAY,
inline_size, (uint8_t *)eip->zei_bits_cleared,
NULL);
} else {
fm_payload_set(ereport,
FM_EREPORT_PAYLOAD_ZFS_BAD_SET_HISTOGRAM,
DATA_TYPE_UINT8_ARRAY,
NBBY * sizeof (uint64_t), eip->zei_histogram_set,
FM_EREPORT_PAYLOAD_ZFS_BAD_CLEARED_HISTOGRAM,
DATA_TYPE_UINT8_ARRAY,
NBBY * sizeof (uint64_t), eip->zei_histogram_cleared,
NULL);
}
return (eip);
}
Expand Down

0 comments on commit cf2a225

Please sign in to comment.