[LTP] [PATCH v3] syscalls/fanotify07: Add stop_children() to cleanup()

[LTP] [PATCH v3] syscalls/fanotify07: Add stop_children() to cleanup()

[Zhao Gongyi via ltp]

When we run the testcase simultaneously, and the ulimit of open
files is small, the testcase will fail and remain many while(1)
tasks in system, it makes the system very stuck.

Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
v2->v3: replace memset() with assignment statement in loop

 testcases/kernel/syscalls/fanotify/fanotify07.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
index cc56d9019..8220213b1 100644
--- a/testcases/kernel/syscalls/fanotify/fanotify07.c
+++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
@@ -86,13 +86,19 @@ static int stop_children(void)
 	int child_ret;
 	int i, ret = 0;

-	for (i = 0; i < MAX_CHILDREN; i++)
+	for (i = 0; i < MAX_CHILDREN; i++) {
+		if (!child_pid[i])
+			continue;
 		SAFE_KILL(child_pid[i], SIGKILL);
+	}

 	for (i = 0; i < MAX_CHILDREN; i++) {
+		if (!child_pid[i])
+			continue;
 		SAFE_WAITPID(child_pid[i], &child_ret, 0);
 		if (!WIFSIGNALED(child_ret))
 			ret = 1;
+		child_pid[i] = 0;
 	}

 	return ret;
@@ -190,6 +196,8 @@ static void setup(void)

 static void cleanup(void)
 {
+	stop_children();
+
 	if (fd_notify > 0)
 		SAFE_CLOSE(fd_notify);
 }
--
2.17.1


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

Re: [LTP] [PATCH v3] syscalls/fanotify07: Add stop_children() to cleanup()

[Matthew Bobrowski via ltp]

On Mon, Nov 22, 2021 at 04:21:46PM +0800, Zhao Gongyi wrote:
> When we run the testcase simultaneously, and the ulimit of open
> files is small, the testcase will fail and remain many while(1)
> tasks in system, it makes the system very stuck.
> 
> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>

This patch looks OK to me. Have you taken the time to verify whether
this fix addresses the issue you've outlined in the patch description?

FTR, please remember to also explicitly CC those who you'd like to get
feedback from. Most of us are exceptionally busy and miss things that
are flying around in the many mailing lists.

> ---
> v2->v3: replace memset() with assignment statement in loop
> 
>  testcases/kernel/syscalls/fanotify/fanotify07.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> --
> 2.17.1
> 
> diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c b/testcases/kernel/syscalls/fanotify/fanotify07.c
> index cc56d9019..8220213b1 100644
> --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> @@ -86,13 +86,19 @@ static int stop_children(void)
>  	int child_ret;
>  	int i, ret = 0;
> 
> -	for (i = 0; i < MAX_CHILDREN; i++)
> +	for (i = 0; i < MAX_CHILDREN; i++) {
> +		if (!child_pid[i])
> +			continue;
>  		SAFE_KILL(child_pid[i], SIGKILL);
> +	}
> 
>  	for (i = 0; i < MAX_CHILDREN; i++) {
> +		if (!child_pid[i])
> +			continue;
>  		SAFE_WAITPID(child_pid[i], &child_ret, 0);
>  		if (!WIFSIGNALED(child_ret))
>  			ret = 1;
> +		child_pid[i] = 0;
>  	}
> 
>  	return ret;
> @@ -190,6 +196,8 @@ static void setup(void)
> 
>  static void cleanup(void)
>  {
> +	stop_children();
> +
>  	if (fd_notify > 0)
>  		SAFE_CLOSE(fd_notify);
>  }

/M

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

Re: [LTP] [PATCH v3] syscalls/fanotify07: Add stop_children() to cleanup()

[zhaogongyi via ltp]

Hi,

I have test the patch through setting "ulimit -n 20" and while true;do ./fanotify07 &;echo "";done 

Or just add a tst_brk as follow:

        /*
         * Create and destroy another instance. This may hang if
         * unanswered fanotify events block notification subsystem.
         */
        newfd = setup_instance();

        SAFE_CLOSE(newfd);
        
		/* inject */
		tst_brk(TBROK, "exit abnormally");

        tst_res(TPASS, "second instance destroyed successfully");

For email CC, I have CC to you and Amir yesterday, please check.


Thanks very much!

Gongyi


> 
> On Mon, Nov 22, 2021 at 04:21:46PM +0800, Zhao Gongyi wrote:
> > When we run the testcase simultaneously, and the ulimit of open files
> > is small, the testcase will fail and remain many while(1) tasks in
> > system, it makes the system very stuck.
> >
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> 
> This patch looks OK to me. Have you taken the time to verify whether this
> fix addresses the issue you've outlined in the patch description?
> 
> FTR, please remember to also explicitly CC those who you'd like to get
> feedback from. Most of us are exceptionally busy and miss things that are
> flying around in the many mailing lists.
> 
> > ---
> > v2->v3: replace memset() with assignment statement in loop
> >
> >  testcases/kernel/syscalls/fanotify/fanotify07.c | 10 +++++++++-
> >  1 file changed, 9 insertions(+), 1 deletion(-)
> >
> > --
> > 2.17.1
> >
> > diff --git a/testcases/kernel/syscalls/fanotify/fanotify07.c
> > b/testcases/kernel/syscalls/fanotify/fanotify07.c
> > index cc56d9019..8220213b1 100644
> > --- a/testcases/kernel/syscalls/fanotify/fanotify07.c
> > +++ b/testcases/kernel/syscalls/fanotify/fanotify07.c
> > @@ -86,13 +86,19 @@ static int stop_children(void)
> >  	int child_ret;
> >  	int i, ret = 0;
> >
> > -	for (i = 0; i < MAX_CHILDREN; i++)
> > +	for (i = 0; i < MAX_CHILDREN; i++) {
> > +		if (!child_pid[i])
> > +			continue;
> >  		SAFE_KILL(child_pid[i], SIGKILL);
> > +	}
> >
> >  	for (i = 0; i < MAX_CHILDREN; i++) {
> > +		if (!child_pid[i])
> > +			continue;
> >  		SAFE_WAITPID(child_pid[i], &child_ret, 0);
> >  		if (!WIFSIGNALED(child_ret))
> >  			ret = 1;
> > +		child_pid[i] = 0;
> >  	}
> >
> >  	return ret;
> > @@ -190,6 +196,8 @@ static void setup(void)
> >
> >  static void cleanup(void)
> >  {
> > +	stop_children();
> > +
> >  	if (fd_notify > 0)
> >  		SAFE_CLOSE(fd_notify);
> >  }
> 
> /M

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

Re: [LTP] [PATCH v3] syscalls/fanotify07: Add stop_children() to cleanup()

[Petr Vorel]

Hi all,

> Hi,

> I have test the patch through setting "ulimit -n 20" and while true;do ./fanotify07 &;echo "";done 

> Or just add a tst_brk as follow:

>         /*
>          * Create and destroy another instance. This may hang if
>          * unanswered fanotify events block notification subsystem.
>          */
>         newfd = setup_instance();

>         SAFE_CLOSE(newfd);

> 		/* inject */
> 		tst_brk(TBROK, "exit abnormally");

>         tst_res(TPASS, "second instance destroyed successfully");

OK, thanks for testing! (suppose you've done this for v3).
I'm going to merge it today.

> For email CC, I have CC to you and Amir yesterday, please check.
You send extra mail on older version with info that you posted new version.
More effective is to use --cc parameter on git format-patch or git send-email in
new version (fewer mails + people don't have to search for particular email in
LTP mailing list mailbox).

Also, FYI we have archive on lore: https://lore.kernel.org/ltp/
(patchwork sometimes some mails).

Kind regards,
Petr

> Thanks very much!

> Gongyi

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