| Message ID | 20201119071646.10052-1-yongxin.liu@windriver.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
| Series | [v3] usertools/devbind: fix binding for built-in kernel drivers | expand |
| Context | Check | Description |
|---|---|---|
| ci/iol-mellanox-Performance | success | Performance Testing PASS |
| ci/travis-robot | success | Travis build: passed |
| ci/iol-intel-Performance | success | Performance Testing PASS |
| ci/iol-intel-Functional | fail | Functional Testing issues |
| ci/iol-testing | warning | Testing issues |
| ci/iol-broadcom-Functional | success | Functional Testing PASS |
| ci/iol-broadcom-Performance | success | Performance Testing PASS |
| ci/Intel-compilation | success | Compilation OK |
| ci/checkpatch | success | coding style OK |
On 19-Nov-20 7:16 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> > --- > > 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 | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py > index 99112b7ab..5b34ccd2a 100755 > --- a/usertools/dpdk-devbind.py > +++ b/usertools/dpdk-devbind.py > @@ -181,7 +181,13 @@ def module_is_loaded(module): > > loaded_modules = sysfs_mods > > - return module in sysfs_mods > + # add built-in modules as loaded > + builtin_mods = subprocess.check_output(["cat /lib/modules/$(uname -r)/modules.builtin"], shell=True).splitlines() I'd rather not call shell directly, it's bad practice and is potentially unsafe. The $(uname -r) can be replaced with: import platform release = platform.uname().release # equivalent to $(uname -r) The rest can follow: filename = os.path.join("/lib/modules/", release, "modules.builtin") # read the file contents, split lines, etc. Also, please check if file exists to ensure we don't crash the script. > + for mod in builtin_mods: > + mod_name = os.path.basename(mod.decode("utf8")).split(".ko", 1) os.path.splitext(os.path.basename(...))? > + loaded_modules.append(mod_name[0]) > + > + return module in loaded_modules > > > def check_modules(): >
> -----Original Message----- > From: Burakov, Anatoly <anatoly.burakov@intel.com> > Sent: Thursday, November 19, 2020 20:19 > To: Liu, Yongxin <Yongxin.Liu@windriver.com>; dev@dpdk.org; > thomas@monjalon.net > Subject: Re: [dpdk-dev] [PATCH v3] usertools/devbind: fix binding for > built-in kernel drivers > > > On 19-Nov-20 7:16 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> > > --- > > > > 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 | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py > > index 99112b7ab..5b34ccd2a 100755 > > --- a/usertools/dpdk-devbind.py > > +++ b/usertools/dpdk-devbind.py > > @@ -181,7 +181,13 @@ def module_is_loaded(module): > > > > loaded_modules = sysfs_mods > > > > - return module in sysfs_mods > > + # add built-in modules as loaded > > + builtin_mods = subprocess.check_output(["cat /lib/modules/$(uname > > + -r)/modules.builtin"], shell=True).splitlines() > > I'd rather not call shell directly, it's bad practice and is potentially > unsafe. The $(uname -r) can be replaced with: > > import platform > release = platform.uname().release # equivalent to $(uname -r) > > The rest can follow: > > filename = os.path.join("/lib/modules/", release, > "modules.builtin") > # read the file contents, split lines, etc. > > Also, please check if file exists to ensure we don't crash the script. > > > + for mod in builtin_mods: > > + mod_name = os.path.basename(mod.decode("utf8")).split(".ko", > > + 1) > > os.path.splitext(os.path.basename(...))? Thanks Anatoly for your detailed instruction. v4 has been sent. /Yongxin > > > + loaded_modules.append(mod_name[0]) > > + > > + return module in loaded_modules > > > > > > def check_modules(): > > > > > -- > Thanks, > Anatoly
diff --git a/usertools/dpdk-devbind.py b/usertools/dpdk-devbind.py index 99112b7ab..5b34ccd2a 100755 --- a/usertools/dpdk-devbind.py +++ b/usertools/dpdk-devbind.py @@ -181,7 +181,13 @@ def module_is_loaded(module): loaded_modules = sysfs_mods - return module in sysfs_mods + # add built-in modules as loaded + builtin_mods = subprocess.check_output(["cat /lib/modules/$(uname -r)/modules.builtin"], shell=True).splitlines() + for mod in builtin_mods: + mod_name = os.path.basename(mod.decode("utf8")).split(".ko", 1) + 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> --- 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 | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)