DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2] usertools: add huge page setup script
Date: Fri, 4 Sep 2020 10:22:28 +0100
Message-ID: <20200904092228.GB1627@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20200903224831.5932-1-stephen@networkplumber.org>

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 <stephen@networkplumber.org>
> ---

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(+)

<snip>

> +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()
> -- 



  reply	other threads:[~2020-09-04  9:22 UTC|newest]

Thread overview: 37+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-18 12:39 [dpdk-dev] [RFC] usertools: Replace dpdk-setup with a python curses based script Sarosh Arif
2020-08-18 17:09 ` Stephen Hemminger
2020-09-01 13:30   ` Thomas Monjalon
2020-09-01 16:56     ` [dpdk-dev] [PATCH] usertools: add huge page setup script Stephen Hemminger
2020-09-02  9:47       ` Ferruh Yigit
2020-09-02  9:55       ` Bruce Richardson
2020-09-02 14:50         ` Stephen Hemminger
2020-09-03 22:48       ` [dpdk-dev] [PATCH v2] " Stephen Hemminger
2020-09-04  9:22         ` Bruce Richardson [this message]
2020-09-04 17:18           ` Stephen Hemminger
2020-09-04 14:58         ` Burakov, Anatoly
2020-09-04 15:10           ` Bruce Richardson
2020-09-04 18:35       ` [dpdk-dev] [PATCH] " Stephen Hemminger
2020-09-04 23:13         ` Ferruh Yigit
2020-09-04 23:30           ` Stephen Hemminger
2020-09-05  3:07       ` [dpdk-dev] [PATCH v4] " Stephen Hemminger
2020-09-06  3:42       ` [dpdk-dev] [PATCH v5] " Stephen Hemminger
2020-09-07  8:54         ` Ferruh Yigit
2020-09-07  8:58           ` Bruce Richardson
2020-09-07 17:20             ` Stephen Hemminger
2020-09-08  8:18               ` Bruce Richardson
2020-09-08 14:58                 ` Stephen Hemminger
2020-09-08 21:49             ` Thomas Monjalon
2020-09-08 15:17       ` [dpdk-dev] [PATCH v6] usertools: add a " Stephen Hemminger
2020-09-09 11:46         ` Ferruh Yigit
2020-09-09 19:26         ` Ajit Khaparde
2020-09-09 18:51       ` [dpdk-dev] [PATCH v7] " Stephen Hemminger
2020-09-14 15:31         ` Burakov, Anatoly
2020-10-20 18:01           ` Ferruh Yigit
2020-11-22 21:39             ` Thomas Monjalon
2020-09-24  4:31         ` Stephen Hemminger
2020-11-22 21:30           ` Thomas Monjalon
2020-11-23  0:12             ` Stephen Hemminger
2020-11-24 17:45             ` Stephen Hemminger
2020-11-24 21:37               ` Thomas Monjalon
2020-11-25  9:16                 ` Ferruh Yigit
2020-08-28 12:09 ` [dpdk-dev] [RFC] usertools: Replace dpdk-setup with a python curses based script Morten Brørup

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200904092228.GB1627@bricha3-MOBL.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=stephen@networkplumber.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git