Misc zevent improvements #11822

ghost · 2021-03-31T18:38:01Z

Motivation and Context

Fixing some issues that were identified while looking into zevent handling.

Description

ratelimit_dropped isn't protected by a lock. We must atomically read and clear the value.

Use a fixed default value for zfs_zevent_len_max rather than scaling by CPUs. The lower bound was too low and the upper bound was too high. 512 is a reasonable default value.

How Has This Been Tested?

ZTS sanity

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Performance enhancement (non-breaking change which improves efficiency)
Code cleanup (non-breaking change which makes code smaller or more readable)
Breaking change (fix or feature that would cause existing functionality to change)
Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
Documentation (a change to man pages or other documentation)

Checklist:

My code follows the OpenZFS code style requirements.
I have updated the documentation accordingly.
I have read the contributing document.
I have added tests to cover my changes.
I have run the ZFS Test Suite with this change applied.
All commit messages are properly formatted and contain Signed-off-by.

ratelimit_dropped isn't protected by a lock. Signed-off-by: Ryan Moeller <ryan@iXsystems.com>

The lower bound for this scaling to too low and the upper bound is too high. Use a fixed default length of 512 instead, which is a reasonable value on any system. Signed-off-by: Ryan Moeller <ryan@iXsystems.com>

The lower bound for this scaling to too low and the upper bound is too high. Use a fixed default length of 512 instead, which is a reasonable value on any system. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11822

ratelimit_dropped isn't protected by a lock and is expected to be updated atomically. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11822

The lower bound for this scaling to too low and the upper bound is too high. Use a fixed default length of 512 instead, which is a reasonable value on any system. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes #11822

ratelimit_dropped isn't protected by a lock and is expected to be updated atomically. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11822

The lower bound for this scaling to too low and the upper bound is too high. Use a fixed default length of 512 instead, which is a reasonable value on any system. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Signed-off-by: Ryan Moeller <ryan@iXsystems.com> Closes openzfs#11822

h1z1 · 2021-05-02T23:05:05Z

@freqlabs did you test clearing thousands of events as part of this? Ran into an issue last week where a system was pausing for some reason. Turned out a user set zfs_deadman_ziotime_ms way too low (10ms), which was spamming events. Clearing them (60k+) alone completely froze the host.

ghost · 2021-05-03T15:04:29Z

I'm not sure how you would have thousands of events queued when this sets the max queue length to 512?

h1z1 · 2021-05-04T20:32:46Z

It shouldn't, question was did you run into that as well and would it be resolved along with this change. Spose either way events shouldn't cause the freezing.

Worth noting this thread - #11966

Which I'm locked out of, has the potential to cause similar issues.

ghost · 2021-05-04T21:30:09Z

No I didn't test clearing thousands of events. I don't think these changes will help whatever is going on there, but it will help ensure the reported number of dropped events due to rate limiting is accurate.