Messages in this thread Patch in this message | | | Date | Fri, 5 Apr 2024 11:26:19 -0400 | From | Johannes Weiner <> | Subject | Re: [PATCH v2 2/5] mm: zswap: calculate limits only when updated |
| |
On Fri, Apr 05, 2024 at 05:35:07AM +0000, Yosry Ahmed wrote: > Currently, we calculate the zswap global limit, and potentially the > acceptance threshold in the zswap, in pages in the zswap store path. > This is unnecessary because the values rarely change. > > Instead, precalculate the them when the module parameters are updated, > which should be rare. Since we are adding custom handlers for setting > the percentages now, add proper validation that they are <= 100. > > Suggested-by: Johannes Weiner <hannes@cmpxchg.org> > Signed-off-by: Yosry Ahmed <yosryahmed@google.com>
Nice! Getting that stuff out of the hotpath!
Two comments below:
> @@ -684,6 +703,43 @@ static int zswap_enabled_param_set(const char *val, > return ret; > } > > +static int __zswap_percent_param_set(const char *val, > + const struct kernel_param *kp) > +{ > + unsigned int n; > + int ret; > + > + ret = kstrtouint(val, 10, &n); > + if (ret || n > 100) > + return -EINVAL; > + > + return param_set_uint(val, kp); > +} > + > +static int zswap_max_pool_param_set(const char *val, > + const struct kernel_param *kp) > +{ > + int err = __zswap_percent_param_set(val, kp); > + > + if (!err) { > + zswap_update_max_pages(); > + zswap_update_accept_thr_pages(); > + } > + > + return err; > +} > + > +static int zswap_accept_thr_param_set(const char *val, > + const struct kernel_param *kp) > +{ > + int err = __zswap_percent_param_set(val, kp); > + > + if (!err) > + zswap_update_accept_thr_pages(); > + > + return err; > +}
I think you can keep this simple and just always update both if anything changes for whatever reason. It's an extremely rare event after all. That should cut it from 3 functions to 1.
Note that totalram_pages can also change during memory onlining and offlining. For that you need a memory notifier that also calls that refresh function. It's simple enough, though, check out the code around register_memory_notifier() in drivers/xen/balloon.c.
| |