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

[Wei Gao via ltp <ltp@lists.linux.it>]

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