From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 12213A04BE; Fri, 4 Sep 2020 11:22:40 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 142111C0CA; Fri, 4 Sep 2020 11:22:39 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 404AD1C0C6 for ; Fri, 4 Sep 2020 11:22:37 +0200 (CEST) IronPort-SDR: 8HxGPXW7oZ9vFglx0FETjBNHYOvO/Qn1bkBmYSj/GgZf8oAX+Ybp0pUypdCB9ddG6QSCq7zABn kDdosn21BqFA== X-IronPort-AV: E=McAfee;i="6000,8403,9733"; a="219270022" X-IronPort-AV: E=Sophos;i="5.76,389,1592895600"; d="scan'208";a="219270022" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga005.jf.intel.com ([10.7.209.41]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Sep 2020 02:22:33 -0700 IronPort-SDR: pZP0N9b5loYcs1hin8yBozRV3yCKz50zffh06SA2dj2cr2mh98vM+t7/vucYmM9PRuj6UlRjJL cMN749VaVhaA== X-IronPort-AV: E=Sophos;i="5.76,389,1592895600"; d="scan'208";a="478414506" Received: from bricha3-mobl.ger.corp.intel.com ([10.251.81.45]) by orsmga005-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-SHA; 04 Sep 2020 02:22:32 -0700 Date: Fri, 4 Sep 2020 10:22:28 +0100 From: Bruce Richardson To: Stephen Hemminger Cc: dev@dpdk.org Message-ID: <20200904092228.GB1627@bricha3-MOBL.ger.corp.intel.com> References: <20200901165643.15668-1-stephen@networkplumber.org> <20200903224831.5932-1-stephen@networkplumber.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200903224831.5932-1-stephen@networkplumber.org> Subject: Re: [dpdk-dev] [PATCH v2] usertools: add huge page setup script X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Sep 03, 2020 at 03:48:31PM -0700, Stephen Hemminger wrote: > This is an improved version of the setup of huge pages > bases on earlier DPDK setup. Differences are: > * it autodetects NUMA vs non NUMA > * it allows setting different page sizes > recent kernels support multiple sizes. > * it accepts a parameter in bytes (not pages). > > If necessary the steps of clearing old settings and mounting/umounting > can be done individually. > > > Signed-off-by: Stephen Hemminger > --- Overall looks really good and readable! Thanks for this. Couple of comments inline below. /Bruce > v2 -- rewrite in python > The script is python3 only because supporting older versions > no longer makes any sense. > > usertools/hugepage-setup.py | 317 ++++++++++++++++++++++++++++++++++++ > 1 file changed, 317 insertions(+) > +def set_numa_pages(nr_pages, hugepgsz): > + for n in glob.glob('/sys/devices/system/node/node*/hugepages'): > + path = '{}/hugepages-{}kB'.format(n, hugepgsz) > + if not exists(path): > + sys.exit( > + '{}Kb is not a valid system huge page size'.format(hugepgsz)) > + > + with open(path + '/nr_hugepages', 'w') as f: > + f.write('{}\n'.format(nr_pages)) > + > + > +def set_non_numa_pages(nr_pages, hugepgsz): > + path = '/sys/kernel/mm/hugepages/hugepages-{}kB'.format(hugepgsz) > + if not exists(path): > + sys.exit('{}Kb is not a valid system huge page size'.format(hugepgsz)) > + > + with open(path + '/nr_hugepages', 'w') as f: > + f.write('{}\n'.format(nr_pages)) > + > + > +def set_pages(pages, hugepgsz): > + '''Sets the numberof huge pages to be reserved''' > + if is_numa(): > + set_numa_pages(pages, hugepgsz) > + else: > + set_non_numa_pages(pages, hugepgsz) > + I'm not sure I agree with this behaviour for numa nodes. When a size is specified on a numa system we probably don't want to reserve that size on all nodes. I think one of two other options actually makes more sense: 1. Divide up the allocation equally between all nodes 2. Require the user to specify a numa node for the allocation. Option #2 is best, I think. > + > +def mount_huge(pagesize): > + global hugedir > + cmd = "mount -t hugetlbfs" + hugedir > + if pagesize: > + cmd += ' -o pagesize={}'.format(pagesize) > + cmd += ' nodev {}'.format(hugedir) > + os.system(cmd) > + > + > +def show_mount(): > + mounted = None > + with open('/proc/mounts') as f: > + for line in f: > + fields = line.split() > + if fields[2] != 'hugetlbfs': > + continue > + if not mounted: > + print("Hugepages mounted on:", end=" ") > + mounted = True > + print(fields[1], end=" ") > + if mounted: > + print() > + else: > + print("Hugepages not mounted") > + > + > +def parse_args(): > + '''Parses the command-line arguments given by the user and takes the > + appropriate action for each''' > + global clear_flag > + global show_flag > + global reserve_kb > + global hugepagesize_kb > + global args > + > + if len(sys.argv) <= 1: > + usage() > + sys.exit(0) > + > + try: > + opts, args = getopt.getopt(sys.argv[1:], "r:p:csmu", [ > + "help", "usage", "show", "clear", "setup=", "eserve=", "pagesize=", Typo -> "eserve" > + "mount", "unmount" > + ]) > + except getopt.GetoptError as error: > + print(str(error)) > + print("Run '%s --usage' for further information" % sys.argv[0]) > + sys.exit(1) > + > + for opt, arg in opts: > + if opt == "--help" or opt == "--usage": > + usage() > + sys.exit(0) > + if opt == "--setup": > + clear_flag = True > + unmount_flag = True > + reserve_kb = get_memsize(arg) > + mount_flag = True > + if opt == "--show" or opt == "-s": > + show_flag = True > + if opt == "--clear" or opt == "-c": > + clear_flag = True > + if opt == "--reserve" or opt == "-r": > + reserve_kb = get_memsize(arg) > + if opt == "--pagesize" or opt == "-p": > + hugepagesize_kb = get_memsize(arg) > + if opt == "--unmount" or opt == "-u": > + unmount_flag = True > + if opt == "--mount" or opt == "-m": > + mount_flag = True > + I think the trend in python is to use argparse rather than getopt, though personally I don't have strong feelings about the issue. > + > +def do_arg_actions(): > + '''do the actual action requested by the user''' > + global clear_flag > + global show_flag > + global hugepagesize_kb > + global reserve_kb > + > + if clear_flag: > + clear_pages() > + if unmount_flag: > + os.system("umount " + hugedir) > + if reserve_kb: > + if hugepagesize_kb is None: > + hugepagesize_kb = default_size() > + if reserve_kb % hugepagesize_kb != 0: > + sys.exit('{} is not a multiple of page size {}'.format( > + reserve_kb, hugepagesize_kb)) > + nr_pages = int(reserve_kb / hugepagesize_kb) > + set_pages(nr_pages, hugepagesize_kb) > + if mount_flag: > + mount_huge(hugepagesize_kb * 1024) > + if show_flag: > + show_pages() > + print() > + show_mount() > + > + > +def main(): > + parse_args() > + do_arg_actions() > + > + > +if __name__ == "__main__": > + main() > --