| Message ID | 1308183516-5247-2-git-send-email-lmr@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Lucas, On Wed, Jun 15, 2011 at 09:18:34PM -0300, Lucas Meneghel Rodrigues wrote: > +class THPNotSupportedError(THPError): > + """ > + Thrown when host does not support tansparent hugepages. > + """ > + pass s/tansparent/transparent/ > +class THPWriteConfigError(THPError): > + """ > + Thrown when host does not support tansparent hugepages. > + """ > + pass same as above. > +class TransparentHugePageConfig(object): > + def __init__(self, test, params): > + """ > + Find paths for transparent hugepages and kugepaged configuration. Also, > + back up original host configuration so it can be restored during > + cleanup. > + """ > + self.params = params > + > + RH_THP_PATH = "/sys/kernel/mm/redhat_transparent_hugepage" > + UPSTREAM_THP_PATH = "/sys/kernel/mm/transparent_hugepage" > + if os.path.isdir(RH_THP_PATH): > + self.thp_path = RH_THP_PATH > + elif os.path.isdir(UPSTREAM_THP_PATH): > + self.thp_path = UPSTREAM_THP_PATH > + else: > + raise THPNotSupportedError("System doesn't support transparent " > + "hugepages") > + > + # 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. > + "%s/khugepaged/scan_sleep_millisecs" % self.thp_path: "100", So this is meant to increase khugepaged scan rate compared to the default. > + "%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. > + "/sys/kernel/mm/ksm/run": "1", > + "/proc/sys/vm/nr_hugepages":"0"} nr_hugepages is unlikely to matter. > + 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. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jun 16, 2011 at 01:34:54PM -0300, Lucas Meneghel Rodrigues wrote: > 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. Thanks! BTW, I'm in the process of reviewing the other patches too. > > > > + # 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. Checking the kernel version may be less reliable than reading it back and seeing if it's yes|no or 0|1 (not sure how the stable kernels will work with 3.0.0 etc..), the future numbering seems a bit variable these days. Theoretically checking the version number is better, but I suspect reading it and handling both formats without regard of the kernel version is simpler in the end. But it's definitely up to you. > > > + "%s/khugepaged/scan_sleep_millisecs" % self.thp_path: "100", > > > > So this is meant to increase khugepaged scan rate compared to the > > default. > > Yep. Sounds good! > > > + "/sys/kernel/mm/ksm/run": "1", > > > + "/proc/sys/vm/nr_hugepages":"0"} > > > > nr_hugepages is unlikely to matter. > > Dropping this then. I think you can drop it yes. I think this only could cause hugetlbfs allocation failures if there's some VM backed by hugetlbfs pages that could then fail allocation and they have no fallback like THP. hugetlbfs and THP are completely separated handled, even if they share plenty code for the clear-copy and put_page/get_page/head/tail/compound logics. > > > + 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. If you wait a few seconds (a few seconds should be enough even with heavy loads) khugepaged should disappear after setting enabled=never, and again in a few seconds it should appear after setting enabled=always. So when setting enabled=never, you can wait with a timeout until pgrep throws the CmdError as expected, and print an error if it pgrep keeps succeeding. The opposite when setting enabled=always. This is a bit kernel dependent (for example other kernel daemons always are present even when the feature is disabled, I handled the races in shutting down and turning on the daemon by optimizing it more, but it's not very important to check that the khugepaged goes away, so you may just limit the pgrep check to when enabled=always to verify khugepaged is there, but you should wait a few seconds before failing as it may take a reschedule for khugepaged to start). Thanks, Andrea -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/client/virt/virt_test_setup.py b/client/virt/virt_test_setup.py index 3e1f5b5..792ffe6 100644 --- a/client/virt/virt_test_setup.py +++ b/client/virt/virt_test_setup.py @@ -1,11 +1,152 @@ """ Library to perform pre/post test setup for KVM autotest. """ -import os, logging +import os, logging, re, sre from autotest_lib.client.common_lib import error from autotest_lib.client.bin import utils +class THPError(Exception): + """ + Base exception for Transparent Hugepage setup. + """ + pass + + +class THPNotSupportedError(THPError): + """ + Thrown when host does not support tansparent hugepages. + """ + pass + + +class THPWriteConfigError(THPError): + """ + Thrown when host does not support tansparent hugepages. + """ + pass + + +class THPKhugepagedError(THPError): + """ + Thrown when khugepaged is not behaving as expected. + """ + pass + + +class TransparentHugePageConfig(object): + def __init__(self, test, params): + """ + Find paths for transparent hugepages and kugepaged configuration. Also, + back up original host configuration so it can be restored during + cleanup. + """ + self.params = params + + RH_THP_PATH = "/sys/kernel/mm/redhat_transparent_hugepage" + UPSTREAM_THP_PATH = "/sys/kernel/mm/transparent_hugepage" + if os.path.isdir(RH_THP_PATH): + self.thp_path = RH_THP_PATH + elif os.path.isdir(UPSTREAM_THP_PATH): + self.thp_path = UPSTREAM_THP_PATH + else: + raise THPNotSupportedError("System doesn't support transparent " + "hugepages") + + # test_cfg holds all the desired host config values we want to set + # before THP tests + test_cfg={"%s/defrag" % self.thp_path: "yes", + "%s/enabled" % self.thp_path: "always", + "%s/khugepaged/defrag" % self.thp_path: "yes", + "%s/khugepaged/scan_sleep_millisecs" % self.thp_path: "100", + "%s/khugepaged/pages_to_scan" % self.thp_path: "4096", + "%s/khugepaged/alloc_sleep_millisecs" % self.thp_path: "100", + "/sys/kernel/mm/ksm/run": "1", + "/proc/sys/vm/nr_hugepages":"0"} + 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) + + + def setup(self): + """ + Configure host for testing. Also, check that khugepaged is working as + expected. + """ + self.set_env() + self.khugepaged_test() + + + def cleanup(self): + """: + Restore the host's original configuration after test + """ + for path in self.original_config: + p_file = open(path, 'w') + p_file.write(self.original_config[path]) + p_file.close() + + class HugePageConfig(object): def __init__(self, params): """