On Thu, 2011-06-16 at 17:56 +0200, Andrea Arcangeli wrote:
> Hi Lucas,
Hi Andrea, thanks for the review! Yiqiao is working on the patchset, v3
or v4 will contain the fixes you have pointed out.
> > + # test_cfg holds all the desired host config values we want to set
> > + # before THP tests
> > + test_cfg={"%s/defrag" % self.thp_path: "yes",
>
> upstream >=3.0.0 this would value would be 1 (on 2.6.38 it's still "yes").
>
> > + "%s/enabled" % self.thp_path: "always",
> > + "%s/khugepaged/defrag" % self.thp_path: "yes",
>
> Upstream >=3.0.0 it's 1.
Ok, we'll set the values conditionally depending on the running kernel
version.
> > + "%s/khugepaged/scan_sleep_millisecs" % self.thp_path: "100",
>
> So this is meant to increase khugepaged scan rate compared to the
> default.
Yep.
> > + "%s/khugepaged/pages_to_scan" % self.thp_path: "4096",
> > + "%s/khugepaged/alloc_sleep_millisecs" % self.thp_path: "100",
>
> An allocation failure is not very likely and it's generally better to
> keep this bigger than the scan_sleep_millisecs. I would keep a factor
> of 6 of difference, so you could set it to 600 if scan_sleep_millisecs
> is set to 100.
Ok, will do.
> > + "/sys/kernel/mm/ksm/run": "1",
> > + "/proc/sys/vm/nr_hugepages":"0"}
>
> nr_hugepages is unlikely to matter.
Dropping this then.
> > + if os.path.isfile("%s/khugepaged/enabled" % self.thp_path):
> > + test_cfg["%s/khugepaged/enabled" % self.thp_path] = "always"
> > + if os.path.isfile("%s/khugepaged/max_ptes_none" % self.thp_path):
> > + test_cfg["%s/khugepaged/max_ptes_none" % self.thp_path] = "511"
> > + test_cfg["%s/defrag" % self.thp_path] = "always"
> > +
> > + tmp_list = []
> > + test_config = self.params.get("test_config", None)
> > + if test_config is not None:
> > + tmp_list = re.split(';', test_config)
> > + while len(tmp_list) > 0:
> > + tmp_cfg = tmp_list.pop()
> > + test_cfg[re.split(":", tmp_cfg)[0]] = sre.split(":", tmp_cfg)[1]
> > +
> > + self.original_config = {}
> > + # Save host current config, so we can restore it during cleanup
> > + for path in test_cfg:
> > + param = open(path, 'r').read()
> > + if ("enabled" in param) or ("defrag" in param):
> > + param = re.split("\[|\]", param)[1] + '\n'
> > + self.original_config[path] = param
> > +
> > + self.test_config = test_cfg
> > +
> > +
> > + def set_env(self):
> > + """
> > + Applies test configuration on the host.
> > + """
> > + if self.test_config:
> > + for path in self.test_config.keys():
> > + file(path, 'w').write(self.test_config[path])
> > +
> > +
> > + def set_params(self, path_dict={}, filename="", value=""):
> > + """
> > + Sets the value of a config file for a given path.
> > +
> > + @param path_dict: Dict of files' paths {path : value}
> > + @param filename: Name of file to setup
> > + @param value: Value set to the configuration files
> > + """
> > + for path in path_dict.keys():
> > + if path in filename:
> > + try:
> > + file(path, "w").write(value)
> > + except IOError, e:
> > + raise THPWriteConfigError("Can not set %s to %s: %s" %
> > + (value, filename, e))
> > +
> > +
> > + def khugepaged_test(self):
> > + """
> > + Start, stop and frequency change test for khugepaged.
> > + """
> > + status_list = ["never", "always", "never"]
> > + for status in status_list:
> > + self.set_params(self.test_config, "enabled", status)
> > + try:
> > + utils.run('pgrep khugepaged')
> > + except error.CmdError:
> > + raise THPKhugepagedError("khugepaged can not be set to "
> > + "status %s" % status)
>
> khugepaged will quit when enabled is set to never, but it's not a bug
> (it's to save the memory of the kernel thread stack when THP is
> disabled). So I'm unsure if the pgrep is going to create spurious errors.
Ok, will improve this check then, based on the expected behavior.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
