DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages
@ 2020-11-30 12:45 Sarosh Arif
  2020-11-30 16:43 ` Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sarosh Arif @ 2020-11-30 12:45 UTC (permalink / raw)
  To: sthemmin; +Cc: dev, 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages
  2020-11-30 12:45 [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages Sarosh Arif
@ 2020-11-30 16:43 ` Stephen Hemminger
  2020-12-17 11:16 ` [dpdk-dev] [v2 PATCH] " Sarosh Arif
  2021-01-08  9:06 ` [dpdk-dev] [v3 " Sarosh Arif
  2 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2020-11-30 16:43 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: sthemmin, dev

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)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2020-11-30 12:45 [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages Sarosh Arif
  2020-11-30 16:43 ` Stephen Hemminger
@ 2020-12-17 11:16 ` Sarosh Arif
  2020-12-17 18:15   ` Stephen Hemminger
  2021-01-08  9:06 ` [dpdk-dev] [v3 " Sarosh Arif
  2 siblings, 1 reply; 8+ messages in thread
From: Sarosh Arif @ 2020-12-17 11:16 UTC (permalink / raw)
  To: sthemmin; +Cc: dev, 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2020-12-17 11:16 ` [dpdk-dev] [v2 PATCH] " Sarosh Arif
@ 2020-12-17 18:15   ` Stephen Hemminger
  2021-01-07  8:06     ` Sarosh Arif
  0 siblings, 1 reply; 8+ messages in thread
From: Stephen Hemminger @ 2020-12-17 18:15 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: sthemmin, dev

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2020-12-17 18:15   ` Stephen Hemminger
@ 2021-01-07  8:06     ` Sarosh Arif
  2021-01-07 15:37       ` Stephen Hemminger
  0 siblings, 1 reply; 8+ messages in thread
From: Sarosh Arif @ 2021-01-07  8:06 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: sthemmin, dev

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [v2 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2021-01-07  8:06     ` Sarosh Arif
@ 2021-01-07 15:37       ` Stephen Hemminger
  0 siblings, 0 replies; 8+ messages in thread
From: Stephen Hemminger @ 2021-01-07 15:37 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: sthemmin, dev

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.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [v3 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2020-11-30 12:45 [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages Sarosh Arif
  2020-11-30 16:43 ` Stephen Hemminger
  2020-12-17 11:16 ` [dpdk-dev] [v2 PATCH] " Sarosh Arif
@ 2021-01-08  9:06 ` Sarosh Arif
  2021-02-05 18:24   ` Thomas Monjalon
  2 siblings, 1 reply; 8+ messages in thread
From: Sarosh Arif @ 2021-01-08  9:06 UTC (permalink / raw)
  To: sthemmin; +Cc: dev, 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


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [v3 PATCH] usertools: show an error message if unable to reserve requested hugepages
  2021-01-08  9:06 ` [dpdk-dev] [v3 " Sarosh Arif
@ 2021-02-05 18:24   ` Thomas Monjalon
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Monjalon @ 2021-02-05 18:24 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: sthemmin, dev, stephen

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.




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-02-05 18:24 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-30 12:45 [dpdk-dev] [PATCH] usertools: show an error message if unable to reserve requested hugepages Sarosh Arif
2020-11-30 16:43 ` Stephen Hemminger
2020-12-17 11:16 ` [dpdk-dev] [v2 PATCH] " Sarosh Arif
2020-12-17 18:15   ` Stephen Hemminger
2021-01-07  8:06     ` Sarosh Arif
2021-01-07 15:37       ` Stephen Hemminger
2021-01-08  9:06 ` [dpdk-dev] [v3 " Sarosh Arif
2021-02-05 18:24   ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).