[PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[Eirik Fuller]

https://github.com/beaker-project/restraint/issues/215 describes a file
descriptor leak which revealed the test failure described here.

The 'DSO data reopen' perf test assumes that RLIMIT_NOFILE limits the
number of open file descriptors, but it actually limits newly opened
file descriptors. When the file descriptor limit is reduced, file
descriptors already open remain open regardless of the new limit. This
test failure does not occur if open file descriptors are contiguous,
beginning at zero.

The following command triggers this perf test failure.

perf test 'DSO data reopen' 3>/dev/null 8>/dev/null

This patch determines the file descriptor limit by opening four files
and then closing them. The limit is set to the fourth file descriptor,
leaving only the first three available because any newly opened file
descriptor must be less than the limit.

Signed-off-by: Eirik Fuller <efuller@redhat.com>
---
 tools/perf/tests/dso-data.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
index 627c1aaf1c9e..43e1b01e5afc 100644
--- a/tools/perf/tests/dso-data.c
+++ b/tools/perf/tests/dso-data.c
@@ -308,10 +308,20 @@ int test__dso_data_cache(struct test *test __maybe_unused, int subtest __maybe_u
 	return 0;
 }
 
+static long new_limit(int count)
+{
+	int fd = open("/dev/null", O_RDONLY);
+	long ret = fd;
+	if (count > 0)
+		ret = new_limit(--count);
+	close(fd);
+	return ret;
+}
+
 int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_unused)
 {
 	struct machine machine;
-	long nr_end, nr = open_files_cnt();
+	long nr_end, nr = open_files_cnt(), lim = new_limit(3);
 	int fd, fd_extra;
 
 #define dso_0 (dsos[0])
@@ -334,7 +344,7 @@ int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_
 
 	/* Make sure we are able to open 3 fds anyway */
 	TEST_ASSERT_VAL("failed to set file limit",
-			!set_fd_limit((nr + 3)));
+			!set_fd_limit((lim)));
 
 	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
 
-- 
2.27.0

Re: [PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[Jiri Olsa]

On Fri, Jun 25, 2021 at 10:38:25PM -0400, Eirik Fuller wrote:
> https://github.com/beaker-project/restraint/issues/215 describes a file
> descriptor leak which revealed the test failure described here.
> 
> The 'DSO data reopen' perf test assumes that RLIMIT_NOFILE limits the
> number of open file descriptors, but it actually limits newly opened
> file descriptors. When the file descriptor limit is reduced, file
> descriptors already open remain open regardless of the new limit. This
> test failure does not occur if open file descriptors are contiguous,
> beginning at zero.
> 
> The following command triggers this perf test failure.
> 
> perf test 'DSO data reopen' 3>/dev/null 8>/dev/null
> 
> This patch determines the file descriptor limit by opening four files
> and then closing them. The limit is set to the fourth file descriptor,
> leaving only the first three available because any newly opened file
> descriptor must be less than the limit.
> 
> Signed-off-by: Eirik Fuller <efuller@redhat.com>
> ---
>  tools/perf/tests/dso-data.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> index 627c1aaf1c9e..43e1b01e5afc 100644
> --- a/tools/perf/tests/dso-data.c
> +++ b/tools/perf/tests/dso-data.c
> @@ -308,10 +308,20 @@ int test__dso_data_cache(struct test *test __maybe_unused, int subtest __maybe_u
>  	return 0;
>  }
>  
> +static long new_limit(int count)
> +{
> +	int fd = open("/dev/null", O_RDONLY);
> +	long ret = fd;
> +	if (count > 0)
> +		ret = new_limit(--count);
> +	close(fd);
> +	return ret;
> +}

nice idea.. thanks

Acked-by: Jiri Olsa <jolsa@redhat.com>

jirka

> +
>  int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_unused)
>  {
>  	struct machine machine;
> -	long nr_end, nr = open_files_cnt();
> +	long nr_end, nr = open_files_cnt(), lim = new_limit(3);
>  	int fd, fd_extra;
>  
>  #define dso_0 (dsos[0])
> @@ -334,7 +344,7 @@ int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_
>  
>  	/* Make sure we are able to open 3 fds anyway */
>  	TEST_ASSERT_VAL("failed to set file limit",
> -			!set_fd_limit((nr + 3)));
> +			!set_fd_limit((lim)));
>  
>  	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
>  
> -- 
> 2.27.0
> 

Re: [PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[Michael Petlan]

On Fri, 25 Jun 2021, Eirik Fuller wrote:
> https://github.com/beaker-project/restraint/issues/215 describes a file
> descriptor leak which revealed the test failure described here.
> 
> The 'DSO data reopen' perf test assumes that RLIMIT_NOFILE limits the
> number of open file descriptors, but it actually limits newly opened
> file descriptors. When the file descriptor limit is reduced, file
> descriptors already open remain open regardless of the new limit. This
> test failure does not occur if open file descriptors are contiguous,
> beginning at zero.
> 
> The following command triggers this perf test failure.
> 
> perf test 'DSO data reopen' 3>/dev/null 8>/dev/null
> 
> This patch determines the file descriptor limit by opening four files
> and then closing them. The limit is set to the fourth file descriptor,
> leaving only the first three available because any newly opened file
> descriptor must be less than the limit.
> 
> Signed-off-by: Eirik Fuller <efuller@redhat.com>

Acked-by: Michael Petlan <mpetlan@redhat.com>

> ---
>  tools/perf/tests/dso-data.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> index 627c1aaf1c9e..43e1b01e5afc 100644
> --- a/tools/perf/tests/dso-data.c
> +++ b/tools/perf/tests/dso-data.c
> @@ -308,10 +308,20 @@ int test__dso_data_cache(struct test *test __maybe_unused, int subtest __maybe_u
>  	return 0;
>  }
>  
> +static long new_limit(int count)
> +{
> +	int fd = open("/dev/null", O_RDONLY);
> +	long ret = fd;
> +	if (count > 0)
> +		ret = new_limit(--count);
> +	close(fd);
> +	return ret;
> +}
> +
>  int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_unused)
>  {
>  	struct machine machine;
> -	long nr_end, nr = open_files_cnt();
> +	long nr_end, nr = open_files_cnt(), lim = new_limit(3);
>  	int fd, fd_extra;
>  
>  #define dso_0 (dsos[0])
> @@ -334,7 +344,7 @@ int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_
>  
>  	/* Make sure we are able to open 3 fds anyway */
>  	TEST_ASSERT_VAL("failed to set file limit",
> -			!set_fd_limit((nr + 3)));
> +			!set_fd_limit((lim)));
>  
>  	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
>  
> -- 
> 2.27.0
> 
> 

Re: [PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[Michael Petlan]

On Wed, 30 Jun 2021, Michael Petlan wrote:
> On Fri, 25 Jun 2021, Eirik Fuller wrote:
> > https://github.com/beaker-project/restraint/issues/215 describes a file
> > descriptor leak which revealed the test failure described here.
> > 
> > The 'DSO data reopen' perf test assumes that RLIMIT_NOFILE limits the
> > number of open file descriptors, but it actually limits newly opened
> > file descriptors. When the file descriptor limit is reduced, file
> > descriptors already open remain open regardless of the new limit. This
> > test failure does not occur if open file descriptors are contiguous,
> > beginning at zero.
> > 
> > The following command triggers this perf test failure.
> > 
> > perf test 'DSO data reopen' 3>/dev/null 8>/dev/null
> > 
> > This patch determines the file descriptor limit by opening four files
> > and then closing them. The limit is set to the fourth file descriptor,
> > leaving only the first three available because any newly opened file
> > descriptor must be less than the limit.
> > 
> > Signed-off-by: Eirik Fuller <efuller@redhat.com>
> 
> Acked-by: Michael Petlan <mpetlan@redhat.com>

Hi Arnaldo,
kindly reminder, could this get merged?

Thank you.

> 
> > ---
> >  tools/perf/tests/dso-data.c | 14 ++++++++++++--
> >  1 file changed, 12 insertions(+), 2 deletions(-)
> > 
> > diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> > index 627c1aaf1c9e..43e1b01e5afc 100644
> > --- a/tools/perf/tests/dso-data.c
> > +++ b/tools/perf/tests/dso-data.c
> > @@ -308,10 +308,20 @@ int test__dso_data_cache(struct test *test __maybe_unused, int subtest __maybe_u
> >  	return 0;
> >  }
> >  
> > +static long new_limit(int count)
> > +{
> > +	int fd = open("/dev/null", O_RDONLY);
> > +	long ret = fd;
> > +	if (count > 0)
> > +		ret = new_limit(--count);
> > +	close(fd);
> > +	return ret;
> > +}
> > +
> >  int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_unused)
> >  {
> >  	struct machine machine;
> > -	long nr_end, nr = open_files_cnt();
> > +	long nr_end, nr = open_files_cnt(), lim = new_limit(3);
> >  	int fd, fd_extra;
> >  
> >  #define dso_0 (dsos[0])
> > @@ -334,7 +344,7 @@ int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_
> >  
> >  	/* Make sure we are able to open 3 fds anyway */
> >  	TEST_ASSERT_VAL("failed to set file limit",
> > -			!set_fd_limit((nr + 3)));
> > +			!set_fd_limit((lim)));
> >  
> >  	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
> >  
> > -- 
> > 2.27.0
> > 
> > 
> 
> 

Re: [PATCH] perf test: Handle fd gaps in test__dso_data_reopen

[Arnaldo Carvalho de Melo]

Em Fri, Jul 30, 2021 at 02:19:58PM +0200, Michael Petlan escreveu:
> On Wed, 30 Jun 2021, Michael Petlan wrote:
> > On Fri, 25 Jun 2021, Eirik Fuller wrote:
> > > https://github.com/beaker-project/restraint/issues/215 describes a file
> > > descriptor leak which revealed the test failure described here.
> > > 
> > > The 'DSO data reopen' perf test assumes that RLIMIT_NOFILE limits the
> > > number of open file descriptors, but it actually limits newly opened
> > > file descriptors. When the file descriptor limit is reduced, file
> > > descriptors already open remain open regardless of the new limit. This
> > > test failure does not occur if open file descriptors are contiguous,
> > > beginning at zero.
> > > 
> > > The following command triggers this perf test failure.
> > > 
> > > perf test 'DSO data reopen' 3>/dev/null 8>/dev/null
> > > 
> > > This patch determines the file descriptor limit by opening four files
> > > and then closing them. The limit is set to the fourth file descriptor,
> > > leaving only the first three available because any newly opened file
> > > descriptor must be less than the limit.
> > > 
> > > Signed-off-by: Eirik Fuller <efuller@redhat.com>
> > 
> > Acked-by: Michael Petlan <mpetlan@redhat.com>
> 
> Hi Arnaldo,
> kindly reminder, could this get merged?

Thanks, applied. Sorry for the delay.

- Arnaldo

 
> Thank you.
> 
> > 
> > > ---
> > >  tools/perf/tests/dso-data.c | 14 ++++++++++++--
> > >  1 file changed, 12 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> > > index 627c1aaf1c9e..43e1b01e5afc 100644
> > > --- a/tools/perf/tests/dso-data.c
> > > +++ b/tools/perf/tests/dso-data.c
> > > @@ -308,10 +308,20 @@ int test__dso_data_cache(struct test *test __maybe_unused, int subtest __maybe_u
> > >  	return 0;
> > >  }
> > >  
> > > +static long new_limit(int count)
> > > +{
> > > +	int fd = open("/dev/null", O_RDONLY);
> > > +	long ret = fd;
> > > +	if (count > 0)
> > > +		ret = new_limit(--count);
> > > +	close(fd);
> > > +	return ret;
> > > +}
> > > +
> > >  int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_unused)
> > >  {
> > >  	struct machine machine;
> > > -	long nr_end, nr = open_files_cnt();
> > > +	long nr_end, nr = open_files_cnt(), lim = new_limit(3);
> > >  	int fd, fd_extra;
> > >  
> > >  #define dso_0 (dsos[0])
> > > @@ -334,7 +344,7 @@ int test__dso_data_reopen(struct test *test __maybe_unused, int subtest __maybe_
> > >  
> > >  	/* Make sure we are able to open 3 fds anyway */
> > >  	TEST_ASSERT_VAL("failed to set file limit",
> > > -			!set_fd_limit((nr + 3)));
> > > +			!set_fd_limit((lim)));
> > >  
> > >  	TEST_ASSERT_VAL("failed to create dsos\n", !dsos__create(3, TEST_FILE_SIZE));
> > >  
> > > -- 
> > > 2.27.0
> > > 
> > > 
> > 
> > 
> 

-- 

- Arnaldo