| Message ID | 20201202110654.20923-1-sarosh.arif@emumba.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | [v2] usertools: show valid hugepage sizes if user requests an invalid hugepage size | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-testing | success | Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/travis-robot | success | Travis build: passed |
| ci/checkpatch | success | coding style OK |
02/12/2020 12:06, Sarosh Arif: > If user requests a hugepage size which is not supported by the system, > currently user gets an error message saying that the requested size > is not a valid system huge page size. In addition to this if we display > the valid hugepage sizes it will be convenient for the user to request > the right size next time. > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com> > --- > v2: > pass string in sys.exit() to remove pylint warning Stephen, is this version OK? > +def get_valid_page_sizes(path): > + valid_page_sizes = "" > + hugepage_dir_path = os.path.split(path)[0] Sorry for not following. Why this split? Maybe add a comment? > + hugepage_dirs = os.listdir(hugepage_dir_path) > + for each_dir in hugepage_dirs: > + hugepage_size = each_dir.split("-")[1] > + valid_page_sizes = valid_page_sizes + " " + hugepage_size Why not using += here? > + return valid_page_sizes > > def set_hugepages(path, pages): > '''Write the number of reserved huge pages''' > @@ -59,10 +67,8 @@ def set_hugepages(path, pages): > except PermissionError: > sys.exit('Permission denied: need to be root!') > except FileNotFoundError: > - filename = os.path.basename(path) > - size = filename[10:] > - sys.exit('{} is not a valid system huge page size'.format(size)) > - > + sys.exit("Invalid page size. Valid page sizes: {}".format( > + get_valid_page_sizes(path)))
On Fri, 05 Feb 2021 19:17:52 +0100 Thomas Monjalon <thomas@monjalon.net> wrote: > 02/12/2020 12:06, Sarosh Arif: > > If user requests a hugepage size which is not supported by the system, > > currently user gets an error message saying that the requested size > > is not a valid system huge page size. In addition to this if we display > > the valid hugepage sizes it will be convenient for the user to request > > the right size next time. > > > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com> > > --- > > v2: > > pass string in sys.exit() to remove pylint warning > > Stephen, is this version OK? This looks good. > > > +def get_valid_page_sizes(path): > > + valid_page_sizes = "" > > + hugepage_dir_path = os.path.split(path)[0] > > Sorry for not following. Why this split? > Maybe add a comment? Guess the code here is using split ad equivalent of dirname() on the path. The path is the string which holds the full filename of the attempted write to sysfs. So this is to get the directory part which could be numa or non-numa. > > > + hugepage_dirs = os.listdir(hugepage_dir_path) > > + for each_dir in hugepage_dirs: > > + hugepage_size = each_dir.split("-")[1] > > + valid_page_sizes = valid_page_sizes + " " + hugepage_size > > Why not using += here? Should work. Overall, this reads a bit like C code here. A python wizard would use list comprehensions here https://docs.python.org/2/tutorial/datastructures.html#list-comprehensions > > > + return valid_page_sizes > > > > def set_hugepages(path, pages): > > '''Write the number of reserved huge pages''' > > @@ -59,10 +67,8 @@ def set_hugepages(path, pages): > > except PermissionError: > > sys.exit('Permission denied: need to be root!') > > except FileNotFoundError: > > - filename = os.path.basename(path) > > - size = filename[10:] > > - sys.exit('{} is not a valid system huge page size'.format(size)) > > - > > + sys.exit("Invalid page size. Valid page sizes: {}".format( > > + get_valid_page_sizes(path))) > > >
On 02-Dec-20 11:06 AM, Sarosh Arif wrote: > If user requests a hugepage size which is not supported by the system, > currently user gets an error message saying that the requested size > is not a valid system huge page size. In addition to this if we display > the valid hugepage sizes it will be convenient for the user to request > the right size next time. > > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com> > --- > v2: > pass string in sys.exit() to remove pylint warning > --- > usertools/dpdk-hugepages.py | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py > index 1be100ca3..09d35bf55 100755 > --- a/usertools/dpdk-hugepages.py > +++ b/usertools/dpdk-hugepages.py > @@ -49,6 +49,14 @@ def get_hugepages(path): > return int(nr_hugepages.read()) > return 0 > > +def get_valid_page_sizes(path): > + valid_page_sizes = "" > + hugepage_dir_path = os.path.split(path)[0] os.path.dirname() ? > + hugepage_dirs = os.listdir(hugepage_dir_path) > + for each_dir in hugepage_dirs: > + hugepage_size = each_dir.split("-")[1] > + valid_page_sizes = valid_page_sizes + " " + hugepage_size You could rewrite this to be clearer and more concise: def get_valid_page_sizes(path): dir = os.path.dirname(path) pg_sizes = (d.split("-")[1] for d in os.listdir(dir)) return " ".join(pg_sizes) > + return valid_page_sizes > > def set_hugepages(path, pages): > '''Write the number of reserved huge pages''' > @@ -59,10 +67,8 @@ def set_hugepages(path, pages): > except PermissionError: > sys.exit('Permission denied: need to be root!') > except FileNotFoundError: > - filename = os.path.basename(path) > - size = filename[10:] > - sys.exit('{} is not a valid system huge page size'.format(size)) > - > + sys.exit("Invalid page size. Valid page sizes: {}".format( > + get_valid_page_sizes(path))) > > def show_numa_pages(): > '''Show huge page reservations on Numa system''' >
diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py index 1be100ca3..09d35bf55 100755 --- a/usertools/dpdk-hugepages.py +++ b/usertools/dpdk-hugepages.py @@ -49,6 +49,14 @@ def get_hugepages(path): return int(nr_hugepages.read()) return 0 +def get_valid_page_sizes(path): + valid_page_sizes = "" + hugepage_dir_path = os.path.split(path)[0] + hugepage_dirs = os.listdir(hugepage_dir_path) + for each_dir in hugepage_dirs: + hugepage_size = each_dir.split("-")[1] + valid_page_sizes = valid_page_sizes + " " + hugepage_size + return valid_page_sizes def set_hugepages(path, pages): '''Write the number of reserved huge pages''' @@ -59,10 +67,8 @@ def set_hugepages(path, pages): except PermissionError: sys.exit('Permission denied: need to be root!') except FileNotFoundError: - filename = os.path.basename(path) - size = filename[10:] - sys.exit('{} is not a valid system huge page size'.format(size)) - + sys.exit("Invalid page size. Valid page sizes: {}".format( + get_valid_page_sizes(path))) def show_numa_pages(): '''Show huge page reservations on Numa system'''
If user requests a hugepage size which is not supported by the system, currently user gets an error message saying that the requested size is not a valid system huge page size. In addition to this if we display the valid hugepage sizes it will be convenient for the user to request the right size next time. Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com> --- v2: pass string in sys.exit() to remove pylint warning --- usertools/dpdk-hugepages.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-)