| Message ID | 20201120022233.25498-1-yongxin.liu@windriver.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | [v4] usertools/devbind: fix binding for built-in kernel drivers | expand |
| Context | Check | Description |
|---|---|---|
| ci/Intel-compilation | fail | apply issues |
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | success | Functional Testing PASS |
| ci/iol-testing | warning | Testing issues |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/travis-robot | success | Travis build: passed |
| ci/checkpatch | success | coding style OK |
On 20-Nov-20 2:22 AM, Yongxin Liu wrote: > A driver can be loaded as a dynamic module or a built-in module. > In commit 681a67288655 ("usertools: check if module is loaded > before binding"), script only checks modules in /sys/module/. > > However, for built-in kernel driver, it only shows up in /sys/module/, > if it has a version or at least one parameter. So add check for > modules in /lib/modules/$(uname -r)/modules.builtin. > > Thanks for Anatoly Burakov's advice. > > Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com> > --- > > v4: > - Replace shell call with platform.uname(). Check file existence > before reading. > > v3: > - Add built-in module list in loaded_modules for checking > instead of removing error check. > > v2: > - fix git commit description style in commit log > - fix typo spelling > > --- > usertools/dpdk-devbind.py | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py > index 99112b7ab..06721709c 100755 > --- a/usertools/dpdk-devbind.py > +++ b/usertools/dpdk-devbind.py > @@ -7,6 +7,7 @@ > import os > import getopt > import subprocess > +import platform > from glob import glob > from os.path import exists, abspath, dirname, basename > from os.path import join as path_join > @@ -181,7 +182,23 @@ def module_is_loaded(module): > > loaded_modules = sysfs_mods > > - return module in sysfs_mods > + # add built-in modules as loaded > + release = platform.uname().release > + filename = os.path.join("/lib/modules/", release, "modules.builtin") > + if os.path.exists(filename): > + try: > + f = open(filename, "r") > + except: > + print("Error: cannot open %s" % filename) > + return > + > + builtin_mods = f.readlines() > + > + for mod in builtin_mods: > + mod_name = os.path.splitext(os.path.basename(mod)) > + loaded_modules.append(mod_name[0]) > + You're not returning a value in error case, this would cause error in the caller of this function. Also, i'd avoid reading the entire file into memory, instead I'd do something like this: try: with open(filename) as f: loaded_modules += [os.path.splitext(os.path.basename(mod)[0] for mod in f] except IOError: print("Warning: cannot read list of built-in kernel modules") # continue with regular code This will be faster, more and readable as well :) > + return module in loaded_modules > > > def check_modules(): >
On 20-Nov-20 2:22 AM, Yongxin Liu wrote: > A driver can be loaded as a dynamic module or a built-in module. > In commit 681a67288655 ("usertools: check if module is loaded > before binding"), script only checks modules in /sys/module/. > > However, for built-in kernel driver, it only shows up in /sys/module/, > if it has a version or at least one parameter. So add check for > modules in /lib/modules/$(uname -r)/modules.builtin. > > Thanks for Anatoly Burakov's advice. Also, no need to add this last line to the commit log :)
diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index 99112b7ab..06721709c 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -7,6 +7,7 @@ import os import getopt import subprocess +import platform from glob import glob from os.path import exists, abspath, dirname, basename from os.path import join as path_join @@ -181,7 +182,23 @@ def module_is_loaded(module): loaded_modules = sysfs_mods - return module in sysfs_mods + # add built-in modules as loaded + release = platform.uname().release + filename = os.path.join("/lib/modules/", release, "modules.builtin") + if os.path.exists(filename): + try: + f = open(filename, "r") + except: + print("Error: cannot open %s" % filename) + return + + builtin_mods = f.readlines() + + for mod in builtin_mods: + mod_name = os.path.splitext(os.path.basename(mod)) + loaded_modules.append(mod_name[0]) + + return module in loaded_modules def check_modules():
A driver can be loaded as a dynamic module or a built-in module. In commit 681a67288655 ("usertools: check if module is loaded before binding"), script only checks modules in /sys/module/. However, for built-in kernel driver, it only shows up in /sys/module/, if it has a version or at least one parameter. So add check for modules in /lib/modules/$(uname -r)/modules.builtin. Thanks for Anatoly Burakov's advice. Signed-off-by: Yongxin Liu <yongxin.liu@windriver.com> --- v4: - Replace shell call with platform.uname(). Check file existence before reading. v3: - Add built-in module list in loaded_modules for checking instead of removing error check. v2: - fix git commit description style in commit log - fix typo spelling --- usertools/dpdk-devbind.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)