From patchwork Mon Jul 27 21:05:26 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Lucas Meneghel Rodrigues X-Patchwork-Id: 37623 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n6RL6ITt027813 for ; Mon, 27 Jul 2009 21:06:18 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754811AbZG0VF3 (ORCPT ); Mon, 27 Jul 2009 17:05:29 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754787AbZG0VF2 (ORCPT ); Mon, 27 Jul 2009 17:05:28 -0400 Received: from qw-out-2122.google.com ([74.125.92.27]:13440 "EHLO qw-out-2122.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754810AbZG0VF1 convert rfc822-to-8bit (ORCPT ); Mon, 27 Jul 2009 17:05:27 -0400 Received: by qw-out-2122.google.com with SMTP id 8so1802806qwh.37 for ; Mon, 27 Jul 2009 14:05:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:sender:received:in-reply-to :references:date:x-google-sender-auth:message-id:subject:from:to:cc :content-type:content-transfer-encoding; bh=Lz+IPQzW7Vkcey6tGqGksca5LwkwmXeohfHjqW1lsH4=; b=OzQtJsISQOLuyyywtrfeoyQSiWXmpgOrNDYxizY9a34AtndlLAdVzmtnwfmg8YgdqZ ixO4Afpr3lNLpXsq9lMw+mXqO7tUy0StObvdxIssZHZtLImTyElQklkjYkIXmk1Y+mj5 0QluCANhq9l1b5lWpFX6XufojNV0dv9AqykIM= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=oouqlCtl2LfZuYAGtfsG9UqlZvc/vKVHcXVmdF3s5COpA7FPcTcRwAX0u7Tmt1uqUK rvD37bv8Nv0wLfAY34O6igVp3HPCqtNqwaS524IzK9ZbSdupcR/ES2EJa0u7v+tYc4uY VK1hEOnv6QwqjTbJGZMmkKiUVjNhCqmy0TLkg= MIME-Version: 1.0 Received: by 10.220.75.9 with SMTP id w9mr4198419vcj.82.1248728726905; Mon, 27 Jul 2009 14:05:26 -0700 (PDT) In-Reply-To: <4A65E6FB.1090901@redhat.com> References: <4A55B759.5080302@redhat.com> <4A57118F.3030907@redhat.com> <1248094728.5318.18.camel@localhost.localdomain> <4A65E6FB.1090901@redhat.com> Date: Mon, 27 Jul 2009 18:05:26 -0300 X-Google-Sender-Auth: ad3d247b1c7df5a4 Message-ID: <6ac58f4f0907271405j2bf6f0fdqe111426120c10146@mail.gmail.com> Subject: Re: [Autotest] [KVM_AUTOTEST] add kvm hugepage variant From: Lucas Meneghel Rodrigues To: =?UTF-8?B?THVrw6HFoSBEb2t0b3I=?= Cc: Autotest mailing list , Jason Wang , KVM list Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Tue, Jul 21, 2009 at 1:04 PM, Lukáš Doktor wrote: > Well, thank you for notifications, I'll keep them in my mind. Ok Lukáš, I have reviewed your patch and have some comments to make: had to change that. logging.debug("VM appears to be alive with PID %d", self.pid) return True diff -Narup a/client/tests/kvm/scripts/hugepage.py b/client/tests/kvm/scripts/ hugepage.py --- a/client/tests/kvm/scripts/hugepage.py 1970-01-01 01:00:00.000000000 +0100 +++ a/client/tests/kvm/scripts/hugepage.py 2009-07-21 16:47:00.000000000 +0200 @@ -0,0 +1,63 @@ +#!/usr/bin/python +# -*- coding: utf-8 -*- +# Alocates enough hugepages and mount hugetlbfs +import os, sys, time + +# Variables check & set +vms = os.environ['KVM_TEST_vms'].split().__len__() +try: + max_vms = int(os.environ['KVM_TEST_max_vms']) +except KeyError: + max_vms = 0 +mem = int(os.environ['KVM_TEST_mem']) +hugepage_path = os.environ['KVM_TEST_hugepage_path'] + +fmeminfo = open("/proc/meminfo", "r") +while fmeminfo: + line = fmeminfo.readline() + if line.startswith("Hugepagesize"): + dumm, hp_size, dumm = line.split() + break +fmeminfo.close() + +if not hp_size: + print "Could not get Hugepagesize from /proc/meminfo file" + raise ValueError Here, is allways good to put the failure reason as one of the parameters to be passed to the exception class constructor, something like: raise ValueError("Could not get Hugepagesize from /proc/meminfo file") + +if vms < max_vms: + vms = max_vms + +vmsm = ((vms * mem) + (vms * 64)) +target = (vmsm * 1024 / int(hp_size)) + +# Iteratively set # of hugepages +fhp = open("/proc/sys/vm/nr_hugepages", "r+") +hp = fhp.readline() +while int(hp) < target: + hp_ = hp + fhp.write(target.__str__()) + fhp.flush() + time.sleep(5) + fhp.seek(0) + hp = int(fhp.readline()) + if hp_ == hp: + raise MemoryError +fhp.close() I liked the above approach to set the hugepage number. +# Mount hugepage filesystem, if necessarily +fmount = open("/proc/mounts", "r") +mount = 1 +line = fmount.readline() +while line: + if line.split()[1] == os.environ['KVM_TEST_hugepage_path']: + mount = 0 + break + line = fmount.readline() +fmount.close() In the above block, it looks to me that we are more interested if we do have a hugetlbfs mount rather than checking if our defined mountpoint is mounted. We need to check in /proc/mounts whether we have a hugetlbfs mount or not instead. +if mount: + if not os.path.exists(hugepage_path): + os.makedirs(hugepage_path) + cmd = "mount -t hugetlbfs none %s" % (hugepage_path) + if os.system(cmd): + raise OSError Same as the above comment when raising exceptions. I think it's better to have our own Error class defined for the script. Since I started fixing the original patch to fit the recent kvm_subprocess commits, I ended up re-creating the patch. I will send it to the mailing list, you tell me what you think. Thanks! Lucas --- 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/tests/kvm/kvm_tests.cfg.sample b/client/tests/kvm/kvm_tests.cfg.sample index 5bd6eb8..70e290d 100644 --- a/client/tests/kvm/kvm_tests.cfg.sample +++ b/client/tests/kvm/kvm_tests.cfg.sample @@ -555,6 +555,13 @@ variants: only default image_format = raw +variants: + - @kvm_smallpages: + - kvm_hugepages: + hugepage_path = /mnt/hugepage + pre_command = "/usr/bin/python scripts/hugepage.py" + extra_params += " -mem-path /mnt/hugepage" Here, I don't think it's necessary to pass /mnt/hugepage as a parameter to the script. We can rather choose a default sensible value and built into the script. variants: - @basic: @@ -568,6 +575,7 @@ variants: only Fedora.8.32 only install setup boot shutdown only rtl8139 + only kvm_smallpages - @sample1: only qcow2 only ide diff --git a/client/tests/kvm/kvm_vm.py b/client/tests/kvm/kvm_vm.py index 48f2916..2b97ccc 100644 --- a/client/tests/kvm/kvm_vm.py +++ b/client/tests/kvm/kvm_vm.py @@ -412,6 +412,13 @@ class VM: self.destroy() return False + if output: + logging.debug("qemu produced some output:\n%s", output) + if "alloc_mem_area" in output: + logging.error("Could not allocate hugepage memory" + " -- qemu command:\n%s", qemu_command) + return False Here seems unnecessary to log every occasion qemu produces output, we should log it only if it contains the pattern we're looking for. Also, with kvm_subprocess recent commits, there's no more output defined. I