Fix most zfs_arc_* mod params not actually being modifiable at runtime #8463
Conversation
Most of the zfs_arc_* module parameters do not have their values used by the ARC code directly. Instead, there is a function, arc_tuning_update, which is called during module initialization and periodically thereafter, whose job is to fetch the module parameter values, clamp/ limit them appropriately, and then assign those values to a separate set of internal variables that are actually referenced by the ARC code. Commit 3ec34e5 featured an overhaul of arc_reclaim_thread, which is the former location where the post-init-time calls to arc_tuning_update would occur. The rework split the work previously done by the arc_reclaim_thread into a pair of replacement threads; and unfortunately, the call to arc_tuning_update fell through the cracks and was lost in the reorganization. This meant that changing almost any ARC-related zfs module parameter via /sys/module/zfs/parameters/ would result in the module parameter value itself appearing to change; however the modification would not actually propagate to the ARC code and have any real effect. This commit reinstates the post-init-time call to arc_tuning_update. It is now called during arc_adjust_cb_check; this should be equivalent to its former call location in arc_reclaim_thread. Signed-off-by: Justin Gottula <justin@jgottula.com>
|
I've now had an opportunity to do a quick test. I used an Arch Linux system that previously had the master branch from ~2 weeks ago installed on it, and I compiled the latest master with this patch incorporated in and tried it out. Setting module parameters like There weren't any crashes or any other indications of anything being majorly wrong. |
Codecov Report
@@ Coverage Diff @@
## master #8463 +/- ##
==========================================
- Coverage 78.62% 78.57% -0.06%
==========================================
Files 380 380
Lines 116002 116003 +1
==========================================
- Hits 91207 91147 -60
- Misses 24795 24856 +61
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Thanks for opening this fix. The timing here is the same as the only version so it should be just as sluggish as before. But if you're feeling ambitious this could be done by switching the relevant module options to use module_param_call instead of module_param. This would allow arc_tuning_update() to be run immediately on demand. A good example of this is the spa_slop_shift module option in module/zfs/spa_misc.c.
|
Since you were able to test by hand, how about adding a ZTS test so that future regressions are avoided? This would be a very simple ZTS test, much simpler than most because there is nothing required other than zfs module load (no need to create pools). If you need help, please ask. |
|
A test case would be good. The best place to put it would be in |
|
I'd suggest we go ahead and merge this as-is to address the immediate issue. We can follow up time permitting with the suggested enhancements. |
behlendorf
added
Status: Accepted
Motivation and Context
Fix for #8405.
Description
Most of the zfs_arc_* module parameters do not have their values used by
the ARC code directly. Instead, there is a function, arc_tuning_update,
which is called during module initialization and periodically
thereafter, whose job is to fetch the module parameter values, clamp/
limit them appropriately, and then assign those values to a separate set
of internal variables that are actually referenced by the ARC code.
Commit 3ec34e5 featured an overhaul of arc_reclaim_thread, which is the
former location where the post-init-time calls to arc_tuning_update
would occur. The rework split the work previously done by the
arc_reclaim_thread into a pair of replacement threads; and
unfortunately, the call to arc_tuning_update fell through the cracks and
was lost in the reorganization.
This meant that changing almost any ARC-related zfs module parameter via
/sys/module/zfs/parameters/ would result in the module parameter value
itself appearing to change; however the modification would not actually
propagate to the ARC code and have any real effect.
This commit reinstates the post-init-time call to arc_tuning_update. It
is now called during arc_adjust_cb_check; this should be equivalent to
its former call location in arc_reclaim_thread.
How Has This Been Tested?
Has not been tested yet.I plan to integrate this into my own zfs builds today or tomorrow to give it a quick test.I've done some brief testing now which looked fine overall; see my comment below.
Types of changes
Checklist:
Signed-off-by.