[LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..bac077fc8 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -33,6 +33,7 @@
 #define NUM_LOOPS	5
 #define NUM_PAGES	32
 #define NUM_PAGES_OFFSET	5
+#define MAX_BUF 4094
 
 /* Needed module to online back memory pages */
 #define HW_MODULE	"hwpoison_inject"
@@ -291,6 +292,31 @@ static void unpoison_this_pfn(unsigned long pfn, int fd)
 	SAFE_WRITE(0, fd, pfn_str, strlen(pfn_str));
 }
 
+static int is_loadable_module(const char *modname)
+{
+	char command[MAX_BUF];
+	char line[MAX_BUF];
+	char *token;
+
+	sprintf(command, "lsmod | grep '^%s'", modname);
+
+	FILE *fp = popen(command, "r");
+
+	if (fp == NULL)
+		tst_brk(TBROK, "Popen command %s failed", command);
+
+	if (fgets(line, MAX_BUF, fp) != NULL) {
+		token = strtok(line, " \t\n");
+		if (strcmp(token, modname) == 0) {
+			pclose(fp);
+			return 1;
+		}
+	}
+
+	pclose(fp);
+	return 0;
+}
+
 /* Find and open the <debugfs>/hwpoison/unpoison-pfn special file */
 static int open_unpoison_pfn(void)
 {
@@ -337,6 +363,7 @@ static void unpoison_pfn(char *begin_tag)
 	unsigned long *pfns;
 	const char *const cmd_rmmod[] = {"rmmod", HW_MODULE, NULL};
 	int found_pfns, fd;
+	int is_loadable = -1;
 
 	pfns = SAFE_MALLOC(sizeof(pfns) * maximum_pfns * run_iterations);
 
@@ -351,8 +378,9 @@ static void unpoison_pfn(char *begin_tag)
 
 		SAFE_CLOSE(fd);
 	}
-	/* remove hwpoison only if we probed it */
-	if (hwpoison_probe)
+	/* remove hwpoison only if we probed it and not built in*/
+	is_loadable = is_loadable_module(HW_MODULE);
+	if (hwpoison_probe && (is_loadable == 1))
 		SAFE_CMD(cmd_rmmod, NULL, NULL);
 }
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> +static int is_loadable_module(const char *modname)
> +{
> +	char command[MAX_BUF];
> +	char line[MAX_BUF];
> +	char *token;
> +
> +	sprintf(command, "lsmod | grep '^%s'", modname);
> +
> +	FILE *fp = popen(command, "r");
> +
> +	if (fp == NULL)
> +		tst_brk(TBROK, "Popen command %s failed", command);
> +
> +	if (fgets(line, MAX_BUF, fp) != NULL) {
> +		token = strtok(line, " \t\n");
> +		if (strcmp(token, modname) == 0) {
> +			pclose(fp);
> +			return 1;
> +		}
> +	}
> +
> +	pclose(fp);
> +	return 0;
> +}

The code already has if (!find_in_file("/proc/modules", HW_MODULE)), you
are reinventing the wheel.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Fri, Mar 10, 2023 at 03:52:16PM +0100, Cyril Hrubis wrote:
> Hi!
> > +static int is_loadable_module(const char *modname)
> > +{
> > +	char command[MAX_BUF];
> > +	char line[MAX_BUF];
> > +	char *token;
> > +
> > +	sprintf(command, "lsmod | grep '^%s'", modname);
> > +
> > +	FILE *fp = popen(command, "r");
> > +
> > +	if (fp == NULL)
> > +		tst_brk(TBROK, "Popen command %s failed", command);
> > +
> > +	if (fgets(line, MAX_BUF, fp) != NULL) {
> > +		token = strtok(line, " \t\n");
> > +		if (strcmp(token, modname) == 0) {
> > +			pclose(fp);
> > +			return 1;
> > +		}
> > +	}
> > +
> > +	pclose(fp);
> > +	return 0;
> > +}
> 
> The code already has if (!find_in_file("/proc/modules", HW_MODULE)), you
> are reinventing the wheel.
There is an issue happen during our test, the fail happen with following failed msg.
rmmod: ERROR: Module hwpoison_inject is builtin.
madvise11.c:356: TWARN: rmmod failed (1)

So i think before rmmod we should check this module can be rmmod or not.
And every modules which can show in lsmod output, it means the module can be unload.
I am not sure the output of /proc/modules contain ONLY loadable module, it maybe
also can contain the buildin module so this function created.

> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Fri, Mar 10, 2023 at 09:23:09PM -0500, Wei Gao via ltp wrote:
> On Fri, Mar 10, 2023 at 03:52:16PM +0100, Cyril Hrubis wrote:
> > Hi!
> > > +static int is_loadable_module(const char *modname)
> > > +{
> > > +	char command[MAX_BUF];
> > > +	char line[MAX_BUF];
> > > +	char *token;
> > > +
> > > +	sprintf(command, "lsmod | grep '^%s'", modname);
> > > +
> > > +	FILE *fp = popen(command, "r");
> > > +
> > > +	if (fp == NULL)
> > > +		tst_brk(TBROK, "Popen command %s failed", command);
> > > +
> > > +	if (fgets(line, MAX_BUF, fp) != NULL) {
> > > +		token = strtok(line, " \t\n");
> > > +		if (strcmp(token, modname) == 0) {
> > > +			pclose(fp);
> > > +			return 1;
> > > +		}
> > > +	}
> > > +
> > > +	pclose(fp);
> > > +	return 0;
> > > +}
> > 
> > The code already has if (!find_in_file("/proc/modules", HW_MODULE)), you
> > are reinventing the wheel.
> There is an issue happen during our test, the fail happen with following failed msg.
> rmmod: ERROR: Module hwpoison_inject is builtin.
> madvise11.c:356: TWARN: rmmod failed (1)
> 
> So i think before rmmod we should check this module can be rmmod or not.
> And every modules which can show in lsmod output, it means the module can be unload.
> I am not sure the output of /proc/modules contain ONLY loadable module, it maybe
> also can contain the buildin module so this function created.
> 
I have found modinfo is more stable way to check whether module is builtin or not.
I have update the patch to v3 now.
> > 
> > -- 
> > Cyril Hrubis
> > chrubis@suse.cz
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> > The code already has if (!find_in_file("/proc/modules", HW_MODULE)), you
> > are reinventing the wheel.
> There is an issue happen during our test, the fail happen with following failed msg.
> rmmod: ERROR: Module hwpoison_inject is builtin.
> madvise11.c:356: TWARN: rmmod failed (1)
> 
> So i think before rmmod we should check this module can be rmmod or not.
> And every modules which can show in lsmod output, it means the module can be unload.
> I am not sure the output of /proc/modules contain ONLY loadable module, it maybe
> also can contain the buildin module so this function created.

The lsmod command reads the /proc/modules, that's where it gets the list
of loaded modules.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
Also description why this is needed is missing from the commit message.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v1] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Fri, Mar 10, 2023 at 03:53:02PM +0100, Cyril Hrubis wrote:
> Hi!
> Also description why this is needed is missing from the commit message.
I have update commit msg with patch v2
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

Following fail msg will popup if we try to rmmod buildin module:
rmmod: ERROR: Module hwpoison_inject is builtin

Before rmmod we should check this module can be rmmod or not.
Every modules which can show in lsmod output, it means the module can be unload.

But for output of /proc/modules i am not sure it can contain ONLY loadable module.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 32 +++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..bac077fc8 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -33,6 +33,7 @@
 #define NUM_LOOPS	5
 #define NUM_PAGES	32
 #define NUM_PAGES_OFFSET	5
+#define MAX_BUF 4094
 
 /* Needed module to online back memory pages */
 #define HW_MODULE	"hwpoison_inject"
@@ -291,6 +292,31 @@ static void unpoison_this_pfn(unsigned long pfn, int fd)
 	SAFE_WRITE(0, fd, pfn_str, strlen(pfn_str));
 }
 
+static int is_loadable_module(const char *modname)
+{
+	char command[MAX_BUF];
+	char line[MAX_BUF];
+	char *token;
+
+	sprintf(command, "lsmod | grep '^%s'", modname);
+
+	FILE *fp = popen(command, "r");
+
+	if (fp == NULL)
+		tst_brk(TBROK, "Popen command %s failed", command);
+
+	if (fgets(line, MAX_BUF, fp) != NULL) {
+		token = strtok(line, " \t\n");
+		if (strcmp(token, modname) == 0) {
+			pclose(fp);
+			return 1;
+		}
+	}
+
+	pclose(fp);
+	return 0;
+}
+
 /* Find and open the <debugfs>/hwpoison/unpoison-pfn special file */
 static int open_unpoison_pfn(void)
 {
@@ -337,6 +363,7 @@ static void unpoison_pfn(char *begin_tag)
 	unsigned long *pfns;
 	const char *const cmd_rmmod[] = {"rmmod", HW_MODULE, NULL};
 	int found_pfns, fd;
+	int is_loadable = -1;
 
 	pfns = SAFE_MALLOC(sizeof(pfns) * maximum_pfns * run_iterations);
 
@@ -351,8 +378,9 @@ static void unpoison_pfn(char *begin_tag)
 
 		SAFE_CLOSE(fd);
 	}
-	/* remove hwpoison only if we probed it */
-	if (hwpoison_probe)
+	/* remove hwpoison only if we probed it and not built in*/
+	is_loadable = is_loadable_module(HW_MODULE);
+	if (hwpoison_probe && (is_loadable == 1))
 		SAFE_CMD(cmd_rmmod, NULL, NULL);
 }
 
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

Following fail msg will popup if we try to rmmod buildin module:
rmmod: ERROR: Module hwpoison_inject is builtin

So need add extra check.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..18e120115 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -33,6 +33,7 @@
 #define NUM_LOOPS	5
 #define NUM_PAGES	32
 #define NUM_PAGES_OFFSET	5
+#define MAX_BUF 4094
 
 /* Needed module to online back memory pages */
 #define HW_MODULE	"hwpoison_inject"
@@ -291,6 +292,29 @@ static void unpoison_this_pfn(unsigned long pfn, int fd)
 	SAFE_WRITE(0, fd, pfn_str, strlen(pfn_str));
 }
 
+static int is_builtin(const char *modname)
+{
+	char command[MAX_BUF];
+	char line[MAX_BUF];
+
+	sprintf(command, "modinfo %s | grep '^filename'", modname);
+
+	FILE *fp = popen(command, "r");
+
+	if (fp == NULL)
+		tst_brk(TBROK, "Popen command %s failed", command);
+
+	if (fgets(line, MAX_BUF, fp) != NULL) {
+		if (strstr(line, "builtin")) {
+			pclose(fp);
+			return 1;
+		}
+	}
+
+	pclose(fp);
+	return 0;
+}
+
 /* Find and open the <debugfs>/hwpoison/unpoison-pfn special file */
 static int open_unpoison_pfn(void)
 {
@@ -300,7 +324,7 @@ static int open_unpoison_pfn(void)
 	struct mntent *mnt;
 	FILE *mntf;
 
-	if (!find_in_file("/proc/modules", HW_MODULE))
+	if (!find_in_file("/proc/modules", HW_MODULE) && !is_builtin(HW_MODULE))
 		hwpoison_probe = 1;
 
 	/* probe hwpoison only if it isn't already there */
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
>  /* Find and open the <debugfs>/hwpoison/unpoison-pfn special file */
>  static int open_unpoison_pfn(void)
>  {
> @@ -300,7 +324,7 @@ static int open_unpoison_pfn(void)
>  	struct mntent *mnt;
>  	FILE *mntf;
>  
> -	if (!find_in_file("/proc/modules", HW_MODULE))
> +	if (!find_in_file("/proc/modules", HW_MODULE) && !is_builtin(HW_MODULE))
>  		hwpoison_probe = 1;

That does not solve the problem completely though, if we have a kernel
where the hwpoinson_inject is set to N in config the test will attempt
to rmmod it and get different error.

I suppose that the easiest solution would be:

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..7c0bef157 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -300,12 +300,12 @@ static int open_unpoison_pfn(void)
        struct mntent *mnt;
        FILE *mntf;

-       if (!find_in_file("/proc/modules", HW_MODULE))
-               hwpoison_probe = 1;
-
        /* probe hwpoison only if it isn't already there */
-       if (hwpoison_probe)
+       if (!find_in_file("/proc/modules", HW_MODULE)) {
                SAFE_CMD(cmd_modprobe, NULL, NULL);
+               if (find_in_file("/proc/modules", HW_MODULE))
+                       hwpoison_probe = 1;
+       }

        /* debugfs mount point */
        mntf = setmntent("/etc/mtab", "r");


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Mon, Mar 13, 2023 at 10:19:39AM +0100, Cyril Hrubis wrote:
> Hi!
> >  /* Find and open the <debugfs>/hwpoison/unpoison-pfn special file */
> >  static int open_unpoison_pfn(void)
> >  {
> > @@ -300,7 +324,7 @@ static int open_unpoison_pfn(void)
> >  	struct mntent *mnt;
> >  	FILE *mntf;
> >  
> > -	if (!find_in_file("/proc/modules", HW_MODULE))
> > +	if (!find_in_file("/proc/modules", HW_MODULE) && !is_builtin(HW_MODULE))
> >  		hwpoison_probe = 1;
> 
> That does not solve the problem completely though, if we have a kernel
> where the hwpoinson_inject is set to N in config the test will attempt
> to rmmod it and get different error.


I have tested on the kernel which set to N in config and the test will report:
tst_test.c:1180: TCONF: hwpoison_inject driver not available

I think it should caused by following configuration of test case:
        .needs_drivers = (const char *const []) {
                HW_MODULE,
                NULL
        },

So the scenario of "N in kernel config" already handled by LTP framework, i have
to say LTP frame work already do a lot of things which i have no idea... xD

> 
> I suppose that the easiest solution would be:
Yes, indeed your solution is more easy and no need extra check function.
I can make your solution as patch v4. 

> 
> diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
> index 7e291d571..7c0bef157 100644
> --- a/testcases/kernel/syscalls/madvise/madvise11.c
> +++ b/testcases/kernel/syscalls/madvise/madvise11.c
> @@ -300,12 +300,12 @@ static int open_unpoison_pfn(void)
>         struct mntent *mnt;
>         FILE *mntf;
> 
> -       if (!find_in_file("/proc/modules", HW_MODULE))
> -               hwpoison_probe = 1;
> -
>         /* probe hwpoison only if it isn't already there */
> -       if (hwpoison_probe)
> +       if (!find_in_file("/proc/modules", HW_MODULE)) {
>                 SAFE_CMD(cmd_modprobe, NULL, NULL);
> +               if (find_in_file("/proc/modules", HW_MODULE))
> +                       hwpoison_probe = 1;
> +       }
> 
>         /* debugfs mount point */
>         mntf = setmntent("/etc/mtab", "r");
> 
> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> > That does not solve the problem completely though, if we have a kernel
> > where the hwpoinson_inject is set to N in config the test will attempt
> > to rmmod it and get different error.
> 
> 
> I have tested on the kernel which set to N in config and the test will report:
> tst_test.c:1180: TCONF: hwpoison_inject driver not available
> 
> I think it should caused by following configuration of test case:
>         .needs_drivers = (const char *const []) {
>                 HW_MODULE,
>                 NULL
>         },
> 
> So the scenario of "N in kernel config" already handled by LTP framework, i have
> to say LTP frame work already do a lot of things which i have no idea... xD

Ah, missed that part as well.

Looking at lib/tst_kernel.c we can also easily add tst_buildin_driver()
function into the LTP library with just:

diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index ecf4b917e..6000522b7 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -153,6 +153,11 @@ static int tst_check_driver_(const char *driver)
        return -1;
 }

+int tst_buildin_driver(const char *driver)
+{
+       return !tst_search_driver(driver, "modules.buildin");
+}
+
 int tst_check_driver(const char *driver)
 {
 #ifdef __ANDROID__


-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Mon, Mar 13, 2023 at 01:37:12PM +0100, Cyril Hrubis wrote:
> Hi!
> > > That does not solve the problem completely though, if we have a kernel
> > > where the hwpoinson_inject is set to N in config the test will attempt
> > > to rmmod it and get different error.
> > 
> > 
> > I have tested on the kernel which set to N in config and the test will report:
> > tst_test.c:1180: TCONF: hwpoison_inject driver not available
> > 
> > I think it should caused by following configuration of test case:
> >         .needs_drivers = (const char *const []) {
> >                 HW_MODULE,
> >                 NULL
> >         },
> > 
> > So the scenario of "N in kernel config" already handled by LTP framework, i have
> > to say LTP frame work already do a lot of things which i have no idea... xD
> 
> Ah, missed that part as well.
> 
> Looking at lib/tst_kernel.c we can also easily add tst_buildin_driver()
> function into the LTP library with just:
> 
> diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
> index ecf4b917e..6000522b7 100644
> --- a/lib/tst_kernel.c
> +++ b/lib/tst_kernel.c
> @@ -153,6 +153,11 @@ static int tst_check_driver_(const char *driver)
>         return -1;
>  }
> 
> +int tst_buildin_driver(const char *driver)
> +{
> +       return !tst_search_driver(driver, "modules.buildin");
> +}
> +
>  int tst_check_driver(const char *driver)
>  {
>  #ifdef __ANDROID__
> 

Try use above implementation but i found another TWARN : (

localhost:/home/ltp/testcases/kernel/syscalls/madvise # ./madvise11
tst_test.c:1560: TINFO: Timeout per run is 0h 01m 00s
madvise11.c:396: TINFO: Spawning 5 threads, with a total of 800 memory pages
madvise11.c:165: TINFO: Thread [3] returned 0, succeeded.
madvise11.c:165: TINFO: Thread [1] returned 0, succeeded.
madvise11.c:165: TINFO: Thread [2] returned 0, succeeded.
madvise11.c:165: TINFO: Thread [4] returned 0, succeeded.
madvise11.c:165: TINFO: Thread [0] returned 0, succeeded.
madvise11.c:199: TPASS: soft-offline / mmap race still clean
madvise11.c:327: TWARN: open(/sys/kernel/debug/hwpoison/unpoison-pfn,1,0000) failed: ENOENT (2) !!!!


> 
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> > +int tst_buildin_driver(const char *driver)
> > +{
> > +       return !tst_search_driver(driver, "modules.buildin");
> > +}
> > +
> >  int tst_check_driver(const char *driver)
> >  {
> >  #ifdef __ANDROID__
> > 
> 
> Try use above implementation but i found another TWARN : (
> 
> localhost:/home/ltp/testcases/kernel/syscalls/madvise # ./madvise11
> tst_test.c:1560: TINFO: Timeout per run is 0h 01m 00s
> madvise11.c:396: TINFO: Spawning 5 threads, with a total of 800 memory pages
> madvise11.c:165: TINFO: Thread [3] returned 0, succeeded.
> madvise11.c:165: TINFO: Thread [1] returned 0, succeeded.
> madvise11.c:165: TINFO: Thread [2] returned 0, succeeded.
> madvise11.c:165: TINFO: Thread [4] returned 0, succeeded.
> madvise11.c:165: TINFO: Thread [0] returned 0, succeeded.
> madvise11.c:199: TPASS: soft-offline / mmap race still clean
> madvise11.c:327: TWARN: open(/sys/kernel/debug/hwpoison/unpoison-pfn,1,0000) failed: ENOENT (2) !!!!

That's strange, this should be equivalent to the modinfo patch you send,
at least the modinfo parses the same file.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Mon, Mar 13, 2023 at 03:06:38PM +0100, Cyril Hrubis wrote:
> Hi!
> > > +int tst_buildin_driver(const char *driver)
> > > +{
> > > +       return !tst_search_driver(driver, "modules.buildin");
> > > +}
> > > +
> > >  int tst_check_driver(const char *driver)
> > >  {
> > >  #ifdef __ANDROID__
> > > 
> > 
> > Try use above implementation but i found another TWARN : (
> > 
> > localhost:/home/ltp/testcases/kernel/syscalls/madvise # ./madvise11
> > tst_test.c:1560: TINFO: Timeout per run is 0h 01m 00s
> > madvise11.c:396: TINFO: Spawning 5 threads, with a total of 800 memory pages
> > madvise11.c:165: TINFO: Thread [3] returned 0, succeeded.
> > madvise11.c:165: TINFO: Thread [1] returned 0, succeeded.
> > madvise11.c:165: TINFO: Thread [2] returned 0, succeeded.
> > madvise11.c:165: TINFO: Thread [4] returned 0, succeeded.
> > madvise11.c:165: TINFO: Thread [0] returned 0, succeeded.
> > madvise11.c:199: TPASS: soft-offline / mmap race still clean
> > madvise11.c:327: TWARN: open(/sys/kernel/debug/hwpoison/unpoison-pfn,1,0000) failed: ENOENT (2) !!!!
> 
> That's strange, this should be equivalent to the modinfo patch you send,
> at least the modinfo parses the same file.
> 


This is caused by "_" and "-", current search function not do this tricky translate part.
Input parameter is hwpoison_inject but actually string in modules.xxx is hwpoison-inject

/lib/modules/5.14.21-150400.24.41-default/modules.dep | grep hwpo
kernel/mm/hwpoison-inject.ko.zst:

Other info just FYI:
//modprobe can accept both "-" and "_"
localhost:/home/ltp # modprobe hwpoison-inject
localhost:/home/ltp # modprobe hwpoison_inject

//get info from lsmod and /proc use "_"
localhost:/home/ltp # lsmod | grep hwpo
hwpoison_inject        16384  0
localhost:/home/ltp # cat /proc/modules | grep hwp
hwpoison_inject 16384 0 - Live 0xffffffffc09d6000

> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> This is caused by "_" and "-", current search function not do this tricky translate part.
> Input parameter is hwpoison_inject but actually string in modules.xxx is hwpoison-inject
> 
> /lib/modules/5.14.21-150400.24.41-default/modules.dep | grep hwpo
> kernel/mm/hwpoison-inject.ko.zst:
> 
> Other info just FYI:
> //modprobe can accept both "-" and "_"
> localhost:/home/ltp # modprobe hwpoison-inject
> localhost:/home/ltp # modprobe hwpoison_inject
> 
> //get info from lsmod and /proc use "_"
> localhost:/home/ltp # lsmod | grep hwpo
> hwpoison_inject        16384  0
> localhost:/home/ltp # cat /proc/modules | grep hwp
> hwpoison_inject 16384 0 - Live 0xffffffffc09d6000

Sounds like a bug that shoudl be fixed, we probably need to create two
search strings, one with dashes and one with underscores and try to
strstr() both.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> This is caused by "_" and "-", current search function not do this tricky translate part.
>> Input parameter is hwpoison_inject but actually string in modules.xxx is hwpoison-inject
>> 
>> /lib/modules/5.14.21-150400.24.41-default/modules.dep | grep hwpo
>> kernel/mm/hwpoison-inject.ko.zst:
>> 
>> Other info just FYI:
>> //modprobe can accept both "-" and "_"
>> localhost:/home/ltp # modprobe hwpoison-inject
>> localhost:/home/ltp # modprobe hwpoison_inject
>> 
>> //get info from lsmod and /proc use "_"
>> localhost:/home/ltp # lsmod | grep hwpo
>> hwpoison_inject        16384  0
>> localhost:/home/ltp # cat /proc/modules | grep hwp
>> hwpoison_inject 16384 0 - Live 0xffffffffc09d6000
>
> Sounds like a bug that shoudl be fixed, we probably need to create two
> search strings, one with dashes and one with underscores and try to
> strstr() both.

Could we not check kconfig for builtin modules?

I suppose this would require an extension to tst_kconfig, but the
information is there already.

>
> -- 
> Cyril Hrubis
> chrubis@suse.cz


-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
> > Sounds like a bug that shoudl be fixed, we probably need to create two
> > search strings, one with dashes and one with underscores and try to
> > strstr() both.
> 
> Could we not check kconfig for builtin modules?

Are we 100% sure that the module-name always translates to
CONFIG_MODULE_NAME?

I would say that we are a bit safer if we use the same files the rest of
the tooling uses e.g. depmod, dracut etc.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> > Sounds like a bug that shoudl be fixed, we probably need to create two
>> > search strings, one with dashes and one with underscores and try to
>> > strstr() both.
>> 
>> Could we not check kconfig for builtin modules?
>
> Are we 100% sure that the module-name always translates to
> CONFIG_MODULE_NAME?

I'm fairly sure this is not the case because I have spent quite some
time trying to match modules with their configuration parameters.

I had not considered trying that, but specifying the module names and
config parameters separately.

>
> I would say that we are a bit safer if we use the same files the rest of
> the tooling uses e.g. depmod, dracut etc.

I suppose that copying the logic from depmod would be safest.

-- 
Thank you,
Richard.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v3] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

On Tue, Mar 14, 2023 at 09:15:05AM +0100, Cyril Hrubis wrote:
> Hi!
> > This is caused by "_" and "-", current search function not do this tricky translate part.
> > Input parameter is hwpoison_inject but actually string in modules.xxx is hwpoison-inject
> > 
> > /lib/modules/5.14.21-150400.24.41-default/modules.dep | grep hwpo
> > kernel/mm/hwpoison-inject.ko.zst:
> > 
> > Other info just FYI:
> > //modprobe can accept both "-" and "_"
> > localhost:/home/ltp # modprobe hwpoison-inject
> > localhost:/home/ltp # modprobe hwpoison_inject
> > 
> > //get info from lsmod and /proc use "_"
> > localhost:/home/ltp # lsmod | grep hwpo
> > hwpoison_inject        16384  0
> > localhost:/home/ltp # cat /proc/modules | grep hwp
> > hwpoison_inject 16384 0 - Live 0xffffffffc09d6000
> 
> Sounds like a bug that shoudl be fixed, we probably need to create two
> search strings, one with dashes and one with underscores and try to
> strstr() both.
> 
I found some logic handle both "_" and "-" in LTP other function such as:

static int tst_search_driver(const char *driver, const char *file)
{

        if (strrchr(driver, '-') || strrchr(driver, '_')) {
                char *driver2 = strdup(driver);
                char *ix = driver2;
                char find = '-', replace = '_';

                if (strrchr(driver, '_')) {
                        find = '_';
                        replace = '-';
                }

                while ((ix = strchr(ix, find)))
                        *ix++ = replace;

                ret = tst_search_driver_(driver2, file);
                free(driver2);
        }


I have sent my latest fix with patch v5(include some small reconstruct/clean work on current exit function)
> -- 
> Cyril Hrubis
> chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

Following fail msg will popup if we try to rmmod buildin module:
rmmod: ERROR: Module hwpoison_inject is builtin

So need add extra check.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..7c0bef157 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -300,12 +300,12 @@ static int open_unpoison_pfn(void)
 	struct mntent *mnt;
 	FILE *mntf;
 
-	if (!find_in_file("/proc/modules", HW_MODULE))
-		hwpoison_probe = 1;
-
 	/* probe hwpoison only if it isn't already there */
-	if (hwpoison_probe)
+	if (!find_in_file("/proc/modules", HW_MODULE)) {
 		SAFE_CMD(cmd_modprobe, NULL, NULL);
+		if (find_in_file("/proc/modules", HW_MODULE))
+			hwpoison_probe = 1;
+	}
 
 	/* debugfs mount point */
 	mntf = setmntent("/etc/mtab", "r");
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v5] madvise11.c:Check loadable module before rmmod

[Wei Gao via ltp]

Following fail msg will popup if we try to rmmod buildin module:
rmmod: ERROR: Module hwpoison_inject is builtin

So need add extra check.

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/tst_kernel.h                          | 10 ++++++
 lib/tst_kernel.c                              | 36 +++++++++++--------
 testcases/kernel/syscalls/madvise/madvise11.c |  2 +-
 3 files changed, 33 insertions(+), 15 deletions(-)

diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index 9e17bb71e..0c1262e2f 100644
--- a/include/tst_kernel.h
+++ b/include/tst_kernel.h
@@ -10,6 +10,16 @@
  */
 int tst_kernel_bits(void);
 
+/*
+ * Checks builtin module for the kernel driver.
+ *
+ * @param driver The name of the driver.
+ * @return Returns 0 if builtin driver
+ * -1 when driver is missing or config file not available.
+ * On Android *always* 0 (always expect the driver is available).
+ */
+int tst_check_builtin_driver(const char *driver);
+
 /**
  * Checks support for the kernel driver.
  *
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index ecf4b917e..be3ec92da 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -90,7 +90,7 @@ int tst_kernel_bits(void)
 	return kernel_bits;
 }
 
-static int tst_search_driver(const char *driver, const char *file)
+static int tst_search_driver_(const char *driver, const char *file)
 {
 	struct stat st;
 	char buf[PATH_MAX];
@@ -144,28 +144,19 @@ static int tst_search_driver(const char *driver, const char *file)
 	return ret;
 }
 
-static int tst_check_driver_(const char *driver)
-{
-	if (!tst_search_driver(driver, "modules.dep") ||
-		!tst_search_driver(driver, "modules.builtin"))
-		return 0;
-
-	return -1;
-}
-
-int tst_check_driver(const char *driver)
+static int tst_search_driver(const char *driver, const char *file)
 {
 #ifdef __ANDROID__
 	/*
 	 * Android may not have properly installed modules.* files. We could
-	 * search modules in /system/lib/modules, but to to determine built-in
+	 * search modules in /system/lib/modules, but to determine built-in
 	 * drivers we need modules.builtin. Therefore assume all drivers are
 	 * available.
 	 */
 	return 0;
 #endif
 
-	if (!tst_check_driver_(driver))
+	if (!tst_search_driver_(driver, file))
 		return 0;
 
 	int ret = -1;
@@ -183,9 +174,26 @@ int tst_check_driver(const char *driver)
 		while ((ix = strchr(ix, find)))
 			*ix++ = replace;
 
-		ret = tst_check_driver_(driver2);
+		ret = tst_search_driver_(driver2, file);
 		free(driver2);
 	}
 
 	return ret;
 }
+
+int tst_check_builtin_driver(const char *driver)
+{
+	if (!tst_search_driver(driver, "modules.builtin"))
+		return 0;
+
+	return -1;
+}
+
+int tst_check_driver(const char *driver)
+{
+	if (!tst_search_driver(driver, "modules.dep") ||
+		!tst_search_driver(driver, "modules.builtin"))
+		return 0;
+
+	return -1;
+}
diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..4d99d5289 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -300,7 +300,7 @@ static int open_unpoison_pfn(void)
 	struct mntent *mnt;
 	FILE *mntf;
 
-	if (!find_in_file("/proc/modules", HW_MODULE))
+	if (!find_in_file("/proc/modules", HW_MODULE) && tst_check_builtin_driver(HW_MODULE))
 		hwpoison_probe = 1;
 
 	/* probe hwpoison only if it isn't already there */
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v5] madvise11.c:Check loadable module before rmmod

[Petr Vorel]

Hi all,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

FYI although it turned out that CONFIG_HWPOISON_INJECT=y in one of our kernels
was a mistake (will be changed to CONFIG_HWPOISON_INJECT=m, which is the default
since kernel ~ 3.14 thus nobody noticed), the test still should work with
CONFIG_HWPOISON_INJECT=y.

BTW I'd personally put madvise11.c change to separate commit.

nit: the title should be:

madvise11: Check if module is loadable before rmmod

> Following fail msg will popup if we try to rmmod buildin module:
> rmmod: ERROR: Module hwpoison_inject is builtin

> So need add extra check.

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/tst_kernel.h                          | 10 ++++++
>  lib/tst_kernel.c                              | 36 +++++++++++--------
>  testcases/kernel/syscalls/madvise/madvise11.c |  2 +-
>  3 files changed, 33 insertions(+), 15 deletions(-)

> diff --git a/include/tst_kernel.h b/include/tst_kernel.h
> index 9e17bb71e..0c1262e2f 100644
> --- a/include/tst_kernel.h
> +++ b/include/tst_kernel.h
> @@ -10,6 +10,16 @@
>   */
>  int tst_kernel_bits(void);

> +/*
> + * Checks builtin module for the kernel driver.
Check if the kernel module is built-in .
> + *
> + * @param driver The name of the driver.
> + * @return Returns 0 if builtin driver
> + * -1 when driver is missing or config file not available.
> + * On Android *always* 0 (always expect the driver is available).
> + */
> +int tst_check_builtin_driver(const char *driver);
> +
>  /**
>   * Checks support for the kernel driver.
Checks support for the kernel module (both built-in and loadable).

I'm willing to do the changes before merge.

Kind regards,
Petr

>   *
> diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
> index ecf4b917e..be3ec92da 100644
> --- a/lib/tst_kernel.c
> +++ b/lib/tst_kernel.c
> @@ -90,7 +90,7 @@ int tst_kernel_bits(void)
>  	return kernel_bits;
>  }

> -static int tst_search_driver(const char *driver, const char *file)
> +static int tst_search_driver_(const char *driver, const char *file)
>  {
>  	struct stat st;
>  	char buf[PATH_MAX];
> @@ -144,28 +144,19 @@ static int tst_search_driver(const char *driver, const char *file)
>  	return ret;
>  }

> -static int tst_check_driver_(const char *driver)
> -{
> -	if (!tst_search_driver(driver, "modules.dep") ||
> -		!tst_search_driver(driver, "modules.builtin"))
> -		return 0;
> -
> -	return -1;
> -}
> -
> -int tst_check_driver(const char *driver)
> +static int tst_search_driver(const char *driver, const char *file)
>  {
>  #ifdef __ANDROID__
>  	/*
>  	 * Android may not have properly installed modules.* files. We could
> -	 * search modules in /system/lib/modules, but to to determine built-in
> +	 * search modules in /system/lib/modules, but to determine built-in
>  	 * drivers we need modules.builtin. Therefore assume all drivers are
>  	 * available.
>  	 */
>  	return 0;
>  #endif

> -	if (!tst_check_driver_(driver))
> +	if (!tst_search_driver_(driver, file))
>  		return 0;

>  	int ret = -1;
> @@ -183,9 +174,26 @@ int tst_check_driver(const char *driver)
>  		while ((ix = strchr(ix, find)))
>  			*ix++ = replace;

> -		ret = tst_check_driver_(driver2);
> +		ret = tst_search_driver_(driver2, file);
>  		free(driver2);
>  	}

>  	return ret;
>  }
> +
> +int tst_check_builtin_driver(const char *driver)
> +{
> +	if (!tst_search_driver(driver, "modules.builtin"))
> +		return 0;
> +
> +	return -1;
> +}
> +
> +int tst_check_driver(const char *driver)
> +{
> +	if (!tst_search_driver(driver, "modules.dep") ||
> +		!tst_search_driver(driver, "modules.builtin"))
> +		return 0;
> +
> +	return -1;
> +}
> diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
> index 7e291d571..4d99d5289 100644
> --- a/testcases/kernel/syscalls/madvise/madvise11.c
> +++ b/testcases/kernel/syscalls/madvise/madvise11.c
> @@ -300,7 +300,7 @@ static int open_unpoison_pfn(void)
>  	struct mntent *mnt;
>  	FILE *mntf;

> -	if (!find_in_file("/proc/modules", HW_MODULE))
> +	if (!find_in_file("/proc/modules", HW_MODULE) && tst_check_builtin_driver(HW_MODULE))
>  		hwpoison_probe = 1;

>  	/* probe hwpoison only if it isn't already there */

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v5] madvise11.c:Check loadable module before rmmod

[Cyril Hrubis]

Hi!
The changes looks good, however I would split the patch into the part
that addes new function into the library and second patch should add the
additional check to the madvise test.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v6 0/2] madvise11: Check if module is loadable before rmmod

[Wei Gao via ltp]

Wei Gao (2):
  tst_kernel: Add function check if the kernel module is built-in
  madvise11: Check if module is loadable before rmmod

 include/tst_kernel.h                          | 14 ++++++--
 lib/tst_kernel.c                              | 36 +++++++++++--------
 testcases/kernel/syscalls/madvise/madvise11.c |  2 +-
 3 files changed, 35 insertions(+), 17 deletions(-)

-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v6 1/2] tst_kernel: Add function check if the kernel module is built-in

[Wei Gao via ltp]

Signed-off-by: Wei Gao <wegao@suse.com>
---
 include/tst_kernel.h | 14 ++++++++++++--
 lib/tst_kernel.c     | 36 ++++++++++++++++++++++--------------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/include/tst_kernel.h b/include/tst_kernel.h
index 9e17bb71e..884a1c7f9 100644
--- a/include/tst_kernel.h
+++ b/include/tst_kernel.h
@@ -10,8 +10,18 @@
  */
 int tst_kernel_bits(void);
 
-/**
- * Checks support for the kernel driver.
+/*
+ * Check if the kernel module is built-in .
+ *
+ * @param driver The name of the driver.
+ * @return Returns 0 if builtin driver
+ * -1 when driver is missing or config file not available.
+ * On Android *always* 0 (always expect the driver is available).
+ */
+int tst_check_builtin_driver(const char *driver);
+
+/*
+ * Checks support for the kernel module (both built-in and loadable).
  *
  * @param driver The name of the driver.
  * @return Returns 0 if the kernel has the driver,
diff --git a/lib/tst_kernel.c b/lib/tst_kernel.c
index ecf4b917e..be3ec92da 100644
--- a/lib/tst_kernel.c
+++ b/lib/tst_kernel.c
@@ -90,7 +90,7 @@ int tst_kernel_bits(void)
 	return kernel_bits;
 }
 
-static int tst_search_driver(const char *driver, const char *file)
+static int tst_search_driver_(const char *driver, const char *file)
 {
 	struct stat st;
 	char buf[PATH_MAX];
@@ -144,28 +144,19 @@ static int tst_search_driver(const char *driver, const char *file)
 	return ret;
 }
 
-static int tst_check_driver_(const char *driver)
-{
-	if (!tst_search_driver(driver, "modules.dep") ||
-		!tst_search_driver(driver, "modules.builtin"))
-		return 0;
-
-	return -1;
-}
-
-int tst_check_driver(const char *driver)
+static int tst_search_driver(const char *driver, const char *file)
 {
 #ifdef __ANDROID__
 	/*
 	 * Android may not have properly installed modules.* files. We could
-	 * search modules in /system/lib/modules, but to to determine built-in
+	 * search modules in /system/lib/modules, but to determine built-in
 	 * drivers we need modules.builtin. Therefore assume all drivers are
 	 * available.
 	 */
 	return 0;
 #endif
 
-	if (!tst_check_driver_(driver))
+	if (!tst_search_driver_(driver, file))
 		return 0;
 
 	int ret = -1;
@@ -183,9 +174,26 @@ int tst_check_driver(const char *driver)
 		while ((ix = strchr(ix, find)))
 			*ix++ = replace;
 
-		ret = tst_check_driver_(driver2);
+		ret = tst_search_driver_(driver2, file);
 		free(driver2);
 	}
 
 	return ret;
 }
+
+int tst_check_builtin_driver(const char *driver)
+{
+	if (!tst_search_driver(driver, "modules.builtin"))
+		return 0;
+
+	return -1;
+}
+
+int tst_check_driver(const char *driver)
+{
+	if (!tst_search_driver(driver, "modules.dep") ||
+		!tst_search_driver(driver, "modules.builtin"))
+		return 0;
+
+	return -1;
+}
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v6 1/2] tst_kernel: Add function check if the kernel module is built-in

[Petr Vorel]

Hi Wei,

> Signed-off-by: Wei Gao <wegao@suse.com>
> ---
>  include/tst_kernel.h | 14 ++++++++++++--
>  lib/tst_kernel.c     | 36 ++++++++++++++++++++++--------------
>  2 files changed, 34 insertions(+), 16 deletions(-)

> diff --git a/include/tst_kernel.h b/include/tst_kernel.h
> index 9e17bb71e..884a1c7f9 100644
> --- a/include/tst_kernel.h
> +++ b/include/tst_kernel.h
> @@ -10,8 +10,18 @@
>   */
>  int tst_kernel_bits(void);

> -/**
> - * Checks support for the kernel driver.
> +/*
> + * Check if the kernel module is built-in .
Patchset merged with removed extra space before dot in the end.

Thank you!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v6 2/2] madvise11: Check if module is loadable before rmmod

[Wei Gao via ltp]

Following fail msg will popup if we try to rmmod buildin module:
rmmod: ERROR: Module hwpoison_inject is builtin

So need add extra check.

Signed-off-by: Wei Gao <wegao@suse.com>
Reviewed-by: Petr Vorel <pvorel@suse.cz>
---
 testcases/kernel/syscalls/madvise/madvise11.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/madvise/madvise11.c b/testcases/kernel/syscalls/madvise/madvise11.c
index 7e291d571..4d99d5289 100644
--- a/testcases/kernel/syscalls/madvise/madvise11.c
+++ b/testcases/kernel/syscalls/madvise/madvise11.c
@@ -300,7 +300,7 @@ static int open_unpoison_pfn(void)
 	struct mntent *mnt;
 	FILE *mntf;
 
-	if (!find_in_file("/proc/modules", HW_MODULE))
+	if (!find_in_file("/proc/modules", HW_MODULE) && tst_check_builtin_driver(HW_MODULE))
 		hwpoison_probe = 1;
 
 	/* probe hwpoison only if it isn't already there */
-- 
2.35.3


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp