[dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages

[dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages

[Sarosh Arif]

Sometimes the system is unable to reserve the requested hugepages because
enough space is not available in the RAM. In that case, currently the
script displays no error message hence the user can be under the delusion
that the hugepages he requested are all successfully reserved. This patch
displays an error message if the pages reserved are different from the
requested pages and shows the actual pages reserved.

Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
 usertools/dpdk-hugepages.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index 1be100ca3..3f6e62c8f 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -62,7 +62,10 @@ def set_hugepages(path, pages):
         filename = os.path.basename(path)
         size = filename[10:]
         sys.exit('{} is not a valid system huge page size'.format(size))
-
+    if get_hugepages(path) != pages:
+        print("Unable to reserve required pages. The pages reserved are:")
+        show_pages()
+        args.show = False

 def show_numa_pages():
     '''Show huge page reservations on Numa system'''
@@ -232,6 +235,8 @@ def main():
         '--setup',
         metavar='SIZE',
         help='setup huge pages by doing clear, unmount, reserve and mount')
+
+    global args
     args = parser.parse_args()
 
     if args.setup:
-- 
2.25.1

Re: [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages

[Stephen Hemminger]

On Mon, 30 Nov 2020 17:45:33 +0500
Sarosh Arif <sarosh.arif@emumba.com> wrote:

> Sometimes the system is unable to reserve the requested hugepages because
> enough space is not available in the RAM. In that case, currently the
> script displays no error message hence the user can be under the delusion
> that the hugepages he requested are all successfully reserved. This patch
> displays an error message if the pages reserved are different from the
> requested pages and shows the actual pages reserved.
> 
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>

This is ok, but this code was written to go through python lint
cleanly. This patch introduces new warnings:


usertools/dpdk-hugepages.py:239:4: W0601: Global variable 'args' undefined at the module level (global-variable-undefined)
usertools/dpdk-hugepages.py:239:4: C0103: Constant name "args" doesn't conform to UPPER_CASE naming style (invalid-name)

[dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Sarosh Arif]

Sometimes the system is unable to reserve the requested hugepages because
enough space is not available in the RAM. In that case, currently the
script displays no error message hence the user can be under the delusion
that the hugepages he requested are all successfully reserved. This patch
displays an error message if the pages reserved are different from the
requested pages and shows the actual pages reserved.


Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
v2:
use a global variable SHOW_HUGEPAGES to remove linter warnings
---
 usertools/dpdk-hugepages.py | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index 1be100ca3..83332c096 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -16,6 +16,8 @@
 # systemd mount point for huge pages
 HUGE_MOUNT = "/dev/hugepages"
 
+# show hugepages flag
+SHOW_HUGEPAGES = False
 
 def fmt_memsize(kb):
     '''Format memory size in kB into conventional format'''
@@ -62,6 +64,10 @@ def set_hugepages(path, pages):
         filename = os.path.basename(path)
         size = filename[10:]
         sys.exit('{} is not a valid system huge page size'.format(size))
+    if get_hugepages(path) != pages:
+        print("Unable to reserve required pages. The pages reserved are:")
+        global SHOW_HUGEPAGES
+        SHOW_HUGEPAGES = True
 
 
 def show_numa_pages():
@@ -233,6 +239,9 @@ def main():
         metavar='SIZE',
         help='setup huge pages by doing clear, unmount, reserve and mount')
     args = parser.parse_args()
+
+    global SHOW_HUGEPAGES
+    SHOW_HUGEPAGES = args.show
 
     if args.setup:
         args.clear = True
@@ -260,7 +269,7 @@ def main():
             int(reserve_kb / pagesize_kb), pagesize_kb, node=args.node)
     if args.mount:
         mount_huge(pagesize_kb, HUGE_MOUNT)
-    if args.show:
+    if SHOW_HUGEPAGES:
         show_pages()
         print()
         show_mount()
-- 
2.25.1

Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Stephen Hemminger]

On Thu, 17 Dec 2020 16:16:16 +0500
Sarosh Arif <sarosh.arif@emumba.com> wrote:

> +    if get_hugepages(path) != pages:
> +        print("Unable to reserve required pages. The pages reserved are:")
> +        global SHOW_HUGEPAGES
> +        SHOW_HUGEPAGES = True


Please don't add global's to this script.

The script is close to being clean according to pylint, and globals
are considered bad style and shouldn't be used.

I would just exit if huge pages could not be setup.
The script should leave it up to the user to do another query about
status if they care about what the result is.

Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Sarosh Arif]

On Thu, Dec 17, 2020 at 11:19 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 17 Dec 2020 16:16:16 +0500
> Sarosh Arif <sarosh.arif@emumba.com> wrote:
>
> > +    if get_hugepages(path) != pages:
> > +        print("Unable to reserve required pages. The pages reserved are:")
> > +        global SHOW_HUGEPAGES
> > +        SHOW_HUGEPAGES = True
>
>
> Please don't add global's to this script.
>
> The script is close to being clean according to pylint, and globals
> are considered bad style and shouldn't be used.
>
> I would just exit if huge pages could not be setup.

How about if we just print a warning message such as "Unable to
reserve required pages" before exiting, in case the pages are not
reserved due to lack of space in RAM? Then leave it upon the user to
query how many pages are actually reserved.
>
> The script should leave it up to the user to do another query about
> status if they care about what the result is.

Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Stephen Hemminger]

On Thu, 7 Jan 2021 13:06:35 +0500
Sarosh Arif <sarosh.arif@emumba.com> wrote:

> On Thu, Dec 17, 2020 at 11:19 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Thu, 17 Dec 2020 16:16:16 +0500
> > Sarosh Arif <sarosh.arif@emumba.com> wrote:
> >  
> > > +    if get_hugepages(path) != pages:
> > > +        print("Unable to reserve required pages. The pages reserved are:")
> > > +        global SHOW_HUGEPAGES
> > > +        SHOW_HUGEPAGES = True  
> >
> >
> > Please don't add global's to this script.
> >
> > The script is close to being clean according to pylint, and globals
> > are considered bad style and shouldn't be used.
> >
> > I would just exit if huge pages could not be setup.  
> 
> How about if we just print a warning message such as "Unable to
> reserve required pages" before exiting, in case the pages are not
> reserved due to lack of space in RAM? Then leave it upon the user to
> query how many pages are actually reserved.
> >
> > The script should leave it up to the user to do another query about
> > status if they care about what the result is.  

Just call sys.exit with a message that is all that is needed.

Or maybe trapping other write errors to sysfs here. Probably the
kernel has already tried to report the error, but the try/except code
is not seeing it.

[dpdk-dev] [v3 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Sarosh Arif]

Sometimes the system is unable to reserve the requested hugepages because
enough space is not available in the RAM. In that case, currently the
script displays no error message hence the user can be under the delusion
that the hugepages requested are all successfully reserved. This patch
displays an error message if the pages reserved are different from the
requested pages.


Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
v2:
use a global variable SHOW_HUGEPAGES to remove linter warnings
v3:
only print an error message if unable to reserve hugepages.
---
 usertools/dpdk-hugepages.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/usertools/dpdk-hugepages.py b/usertools/dpdk-hugepages.py
index 1be100ca3..1523be20a 100755
--- a/usertools/dpdk-hugepages.py
+++ b/usertools/dpdk-hugepages.py
@@ -62,6 +62,8 @@ def set_hugepages(path, pages):
         filename = os.path.basename(path)
         size = filename[10:]
         sys.exit('{} is not a valid system huge page size'.format(size))
+    if get_hugepages(path) != pages:
+        sys.exit("Unable to reserve required pages.")
 
 
 def show_numa_pages():
-- 
2.25.1

Re: [dpdk-dev] [v3 PATCH] usertools: show an error message if unable to reserve requested hugepages

[Thomas Monjalon]

08/01/2021 10:06, Sarosh Arif:
> Sometimes the system is unable to reserve the requested hugepages because
> enough space is not available in the RAM. In that case, currently the
> script displays no error message hence the user can be under the delusion
> that the hugepages requested are all successfully reserved. This patch
> displays an error message if the pages reserved are different from the
> requested pages.
> 
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
> v2:
> use a global variable SHOW_HUGEPAGES to remove linter warnings
> v3:
> only print an error message if unable to reserve hugepages.
> ---
> +    if get_hugepages(path) != pages:
> +        sys.exit("Unable to reserve required pages.")

Replaced double quotes by simple ones.

Applied, thanks.