On 10/19/21 5:07 PM, Junaid Shahid wrote:
On 10/19/21 4:48 PM, Ben Gardon wrote:
On Mon, Oct 18, 2021 at 6:40 PM Junaid Shahid <junaids@xxxxxxxxxx> wrote:
-static int set_nx_huge_pages_recovery_ratio(const char *val, const struct kernel_param *kp)
+static int set_nx_huge_pages_recovery_param(const char *val, const struct kernel_param *kp)
{
- unsigned int old_val;
+ bool was_recovery_enabled, is_recovery_enabled;
int err;
- old_val = nx_huge_pages_recovery_ratio;
+ was_recovery_enabled = nx_huge_pages_recovery_ratio;
+
err = param_set_uint(val, kp);
if (err)
return err;
+ is_recovery_enabled = nx_huge_pages_recovery_ratio;
+
if (READ_ONCE(nx_huge_pages) &&
- !old_val && nx_huge_pages_recovery_ratio) {
+ !was_recovery_enabled && is_recovery_enabled) {
struct kvm *kvm;
mutex_lock(&kvm_lock);
I might be missing something, but it seems like setting
nx_huge_pages_recovery_period_ms through this function won't do
anything special. Is there any reason to use this function for it
versus param_set_uint?
Yes, you are right. The original patch was using a 0 period to mean that recovery is disabled, but v2 no longer does that, so we indeed don't need to handle the period parameter through this function.
Actually, I think that it may be a good idea to still use a slightly modified form of that function. If the period was originally set to a large value and is now set to a small value, then ideally we should wake up the thread rather than having it wait for the original period to expire.
