Re: [PATCH 14/14] perf tests: Add test for closing dso objects on EMFILE error

2014-05-27 Thread Jiri Olsa
On Tue, May 27, 2014 at 10:43:57AM +0900, Namhyung Kim wrote:
> On Thu, 15 May 2014 19:23:35 +0200, Jiri Olsa wrote:
> > Testing that perf properly closes opened dso objects
> > and tries to reopen in case we run out of allowed file
> > descriptors for dso data.
> >
> > Cc: Arnaldo Carvalho de Melo 
> > Cc: Corey Ashford 
> > Cc: David Ahern 
> > Cc: Frederic Weisbecker 
> > Cc: Ingo Molnar 
> > Cc: Jean Pihet 
> > Cc: Namhyung Kim 
> > Cc: Paul Mackerras 
> > Cc: Peter Zijlstra 
> > Signed-off-by: Jiri Olsa 
> > ---
> >  tools/perf/tests/builtin-test.c |  4 +++
> >  tools/perf/tests/dso-data.c | 70 
> > +
> >  tools/perf/tests/tests.h|  1 +
> >  3 files changed, 75 insertions(+)
> >
> > diff --git a/tools/perf/tests/builtin-test.c 
> > b/tools/perf/tests/builtin-test.c
> > index c4d581a..a489cda 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -60,6 +60,10 @@ static struct test {
> > .func = test__dso_data_cache,
> > },
> > {
> > +   .desc = "Test dso data reopen",
> > +   .func = test__dso_data_reopen,
> > +   },
> > +   {
> > .desc = "roundtrip evsel->name check",
> > .func = test__perf_evsel__roundtrip_name_test,
> > },
> > diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> > index 84ab939..ecc8acd 100644
> > --- a/tools/perf/tests/dso-data.c
> > +++ b/tools/perf/tests/dso-data.c
> > @@ -328,3 +328,73 @@ int test__dso_data_cache(void)
> > TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
> > return 0;
> >  }
> > +
> > +int test__dso_data_reopen(void)
> > +{
> > +   struct machine machine;
> > +   long nr = open_files_cnt();
> > +#define BUFSIZE 10
> 
> Looks like a copy-n-paste error.. :)

yea.. ;) I wonder why gcc did not scream about that

SNIP

> > +
> > +   /*
> > +* dso_1 should get closed, because we reached
> > +* the file descriptor limit
> > +*/
> > +   TEST_ASSERT_VAL("failed to close dso_0", dso_1->data.fd == -1);
> 
> s/dso_0/dso_1/
> 
> Btw, I don't see a big difference between this and previous testcase.
> Any chance to merge them into one?

- the first one tests the caching or closing of file descriptors
  after dso__data_close is called
  
- the second one tests that we actually try to close dso objects
  in case we cannot open new dso

thanks,
jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 14/14] perf tests: Add test for closing dso objects on EMFILE error

2014-05-27 Thread Jiri Olsa
On Tue, May 27, 2014 at 10:43:57AM +0900, Namhyung Kim wrote:
 On Thu, 15 May 2014 19:23:35 +0200, Jiri Olsa wrote:
  Testing that perf properly closes opened dso objects
  and tries to reopen in case we run out of allowed file
  descriptors for dso data.
 
  Cc: Arnaldo Carvalho de Melo a...@kernel.org
  Cc: Corey Ashford cjash...@linux.vnet.ibm.com
  Cc: David Ahern dsah...@gmail.com
  Cc: Frederic Weisbecker fweis...@gmail.com
  Cc: Ingo Molnar mi...@kernel.org
  Cc: Jean Pihet jean.pi...@linaro.org
  Cc: Namhyung Kim namhy...@kernel.org
  Cc: Paul Mackerras pau...@samba.org
  Cc: Peter Zijlstra a.p.zijls...@chello.nl
  Signed-off-by: Jiri Olsa jo...@kernel.org
  ---
   tools/perf/tests/builtin-test.c |  4 +++
   tools/perf/tests/dso-data.c | 70 
  +
   tools/perf/tests/tests.h|  1 +
   3 files changed, 75 insertions(+)
 
  diff --git a/tools/perf/tests/builtin-test.c 
  b/tools/perf/tests/builtin-test.c
  index c4d581a..a489cda 100644
  --- a/tools/perf/tests/builtin-test.c
  +++ b/tools/perf/tests/builtin-test.c
  @@ -60,6 +60,10 @@ static struct test {
  .func = test__dso_data_cache,
  },
  {
  +   .desc = Test dso data reopen,
  +   .func = test__dso_data_reopen,
  +   },
  +   {
  .desc = roundtrip evsel-name check,
  .func = test__perf_evsel__roundtrip_name_test,
  },
  diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
  index 84ab939..ecc8acd 100644
  --- a/tools/perf/tests/dso-data.c
  +++ b/tools/perf/tests/dso-data.c
  @@ -328,3 +328,73 @@ int test__dso_data_cache(void)
  TEST_ASSERT_VAL(failed leadking files, nr == open_files_cnt());
  return 0;
   }
  +
  +int test__dso_data_reopen(void)
  +{
  +   struct machine machine;
  +   long nr = open_files_cnt();
  +#define BUFSIZE 10
 
 Looks like a copy-n-paste error.. :)

yea.. ;) I wonder why gcc did not scream about that

SNIP

  +
  +   /*
  +* dso_1 should get closed, because we reached
  +* the file descriptor limit
  +*/
  +   TEST_ASSERT_VAL(failed to close dso_0, dso_1-data.fd == -1);
 
 s/dso_0/dso_1/
 
 Btw, I don't see a big difference between this and previous testcase.
 Any chance to merge them into one?

- the first one tests the caching or closing of file descriptors
  after dso__data_close is called
  
- the second one tests that we actually try to close dso objects
  in case we cannot open new dso

thanks,
jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 14/14] perf tests: Add test for closing dso objects on EMFILE error

2014-05-26 Thread Namhyung Kim
On Thu, 15 May 2014 19:23:35 +0200, Jiri Olsa wrote:
> Testing that perf properly closes opened dso objects
> and tries to reopen in case we run out of allowed file
> descriptors for dso data.
>
> Cc: Arnaldo Carvalho de Melo 
> Cc: Corey Ashford 
> Cc: David Ahern 
> Cc: Frederic Weisbecker 
> Cc: Ingo Molnar 
> Cc: Jean Pihet 
> Cc: Namhyung Kim 
> Cc: Paul Mackerras 
> Cc: Peter Zijlstra 
> Signed-off-by: Jiri Olsa 
> ---
>  tools/perf/tests/builtin-test.c |  4 +++
>  tools/perf/tests/dso-data.c | 70 
> +
>  tools/perf/tests/tests.h|  1 +
>  3 files changed, 75 insertions(+)
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index c4d581a..a489cda 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -60,6 +60,10 @@ static struct test {
>   .func = test__dso_data_cache,
>   },
>   {
> + .desc = "Test dso data reopen",
> + .func = test__dso_data_reopen,
> + },
> + {
>   .desc = "roundtrip evsel->name check",
>   .func = test__perf_evsel__roundtrip_name_test,
>   },
> diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
> index 84ab939..ecc8acd 100644
> --- a/tools/perf/tests/dso-data.c
> +++ b/tools/perf/tests/dso-data.c
> @@ -328,3 +328,73 @@ int test__dso_data_cache(void)
>   TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
>   return 0;
>  }
> +
> +int test__dso_data_reopen(void)
> +{
> + struct machine machine;
> + long nr = open_files_cnt();
> +#define BUFSIZE 10

Looks like a copy-n-paste error.. :)


> + int fd, fd_extra;
> +
> + memset(, 0, sizeof(machine));
> +
> + /*
> +  * Test scenario:
> +  * - create 3 dso objects
> +  * - set process file descriptor limit to current
> +  *   files count + 3
> +  * - test that the first dso gets closed when we
> +  *   reach the files count limit
> +  */
> +
> + /* Make sure we are able to open 3 fds anyway */
> + TEST_ASSERT_VAL("failed to set file limit",
> + !set_fd_limit((nr + 3)));
> +
> + TEST_ASSERT_VAL("failed to create dsos\n", 
> !dsos__create(TEST_FILE_SIZE));
> +
> + /* open dso_0 */
> + fd = dso__data_fd(dso_0, );
> + TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> + /* open dso_1 */
> + fd = dso__data_fd(dso_1, );
> + TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> + /*
> +  * open extra file descriptor and we just
> +  * reached the files count limit
> +  */
> + fd_extra = open("/dev/null", O_RDONLY);
> + TEST_ASSERT_VAL("failed to open extra fd", fd_extra > 0);
> +
> + /* open dso_2 */
> + fd = dso__data_fd(dso_2, );
> + TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> + /*
> +  * dso_0 should get closed, because we reached
> +  * the file descriptor limit
> +  */
> + TEST_ASSERT_VAL("failed to close dso_0", dso_0->data.fd == -1);
> +
> + /* open dso_0 */
> + fd = dso__data_fd(dso_0, );
> + TEST_ASSERT_VAL("failed to get fd", fd > 0);
> +
> + /*
> +  * dso_1 should get closed, because we reached
> +  * the file descriptor limit
> +  */
> + TEST_ASSERT_VAL("failed to close dso_0", dso_1->data.fd == -1);

s/dso_0/dso_1/

Btw, I don't see a big difference between this and previous testcase.
Any chance to merge them into one?

Thanks,
Namhyung

> +
> + /* cleanup everything */
> + close(fd_extra);
> + dsos__delete();
> +
> + pr_debug("nr start %ld, nr stop %ld\n", nr, open_files_cnt());
> +
> + /* Make sure we did not leak any file descriptor. */
> + TEST_ASSERT_VAL("failed leadking files", nr == open_files_cnt());
> + return 0;
> +}
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index 61e12b6..3247ca1 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -29,6 +29,7 @@ int test__pmu(void);
>  int test__attr(void);
>  int test__dso_data(void);
>  int test__dso_data_cache(void);
> +int test__dso_data_reopen(void);
>  int test__parse_events(void);
>  int test__hists_link(void);
>  int test__python_use(void);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH 14/14] perf tests: Add test for closing dso objects on EMFILE error

2014-05-26 Thread Namhyung Kim
On Thu, 15 May 2014 19:23:35 +0200, Jiri Olsa wrote:
 Testing that perf properly closes opened dso objects
 and tries to reopen in case we run out of allowed file
 descriptors for dso data.

 Cc: Arnaldo Carvalho de Melo a...@kernel.org
 Cc: Corey Ashford cjash...@linux.vnet.ibm.com
 Cc: David Ahern dsah...@gmail.com
 Cc: Frederic Weisbecker fweis...@gmail.com
 Cc: Ingo Molnar mi...@kernel.org
 Cc: Jean Pihet jean.pi...@linaro.org
 Cc: Namhyung Kim namhy...@kernel.org
 Cc: Paul Mackerras pau...@samba.org
 Cc: Peter Zijlstra a.p.zijls...@chello.nl
 Signed-off-by: Jiri Olsa jo...@kernel.org
 ---
  tools/perf/tests/builtin-test.c |  4 +++
  tools/perf/tests/dso-data.c | 70 
 +
  tools/perf/tests/tests.h|  1 +
  3 files changed, 75 insertions(+)

 diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
 index c4d581a..a489cda 100644
 --- a/tools/perf/tests/builtin-test.c
 +++ b/tools/perf/tests/builtin-test.c
 @@ -60,6 +60,10 @@ static struct test {
   .func = test__dso_data_cache,
   },
   {
 + .desc = Test dso data reopen,
 + .func = test__dso_data_reopen,
 + },
 + {
   .desc = roundtrip evsel-name check,
   .func = test__perf_evsel__roundtrip_name_test,
   },
 diff --git a/tools/perf/tests/dso-data.c b/tools/perf/tests/dso-data.c
 index 84ab939..ecc8acd 100644
 --- a/tools/perf/tests/dso-data.c
 +++ b/tools/perf/tests/dso-data.c
 @@ -328,3 +328,73 @@ int test__dso_data_cache(void)
   TEST_ASSERT_VAL(failed leadking files, nr == open_files_cnt());
   return 0;
  }
 +
 +int test__dso_data_reopen(void)
 +{
 + struct machine machine;
 + long nr = open_files_cnt();
 +#define BUFSIZE 10

Looks like a copy-n-paste error.. :)


 + int fd, fd_extra;
 +
 + memset(machine, 0, sizeof(machine));
 +
 + /*
 +  * Test scenario:
 +  * - create 3 dso objects
 +  * - set process file descriptor limit to current
 +  *   files count + 3
 +  * - test that the first dso gets closed when we
 +  *   reach the files count limit
 +  */
 +
 + /* Make sure we are able to open 3 fds anyway */
 + TEST_ASSERT_VAL(failed to set file limit,
 + !set_fd_limit((nr + 3)));
 +
 + TEST_ASSERT_VAL(failed to create dsos\n, 
 !dsos__create(TEST_FILE_SIZE));
 +
 + /* open dso_0 */
 + fd = dso__data_fd(dso_0, machine);
 + TEST_ASSERT_VAL(failed to get fd, fd  0);
 +
 + /* open dso_1 */
 + fd = dso__data_fd(dso_1, machine);
 + TEST_ASSERT_VAL(failed to get fd, fd  0);
 +
 + /*
 +  * open extra file descriptor and we just
 +  * reached the files count limit
 +  */
 + fd_extra = open(/dev/null, O_RDONLY);
 + TEST_ASSERT_VAL(failed to open extra fd, fd_extra  0);
 +
 + /* open dso_2 */
 + fd = dso__data_fd(dso_2, machine);
 + TEST_ASSERT_VAL(failed to get fd, fd  0);
 +
 + /*
 +  * dso_0 should get closed, because we reached
 +  * the file descriptor limit
 +  */
 + TEST_ASSERT_VAL(failed to close dso_0, dso_0-data.fd == -1);
 +
 + /* open dso_0 */
 + fd = dso__data_fd(dso_0, machine);
 + TEST_ASSERT_VAL(failed to get fd, fd  0);
 +
 + /*
 +  * dso_1 should get closed, because we reached
 +  * the file descriptor limit
 +  */
 + TEST_ASSERT_VAL(failed to close dso_0, dso_1-data.fd == -1);

s/dso_0/dso_1/

Btw, I don't see a big difference between this and previous testcase.
Any chance to merge them into one?

Thanks,
Namhyung

 +
 + /* cleanup everything */
 + close(fd_extra);
 + dsos__delete();
 +
 + pr_debug(nr start %ld, nr stop %ld\n, nr, open_files_cnt());
 +
 + /* Make sure we did not leak any file descriptor. */
 + TEST_ASSERT_VAL(failed leadking files, nr == open_files_cnt());
 + return 0;
 +}
 diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
 index 61e12b6..3247ca1 100644
 --- a/tools/perf/tests/tests.h
 +++ b/tools/perf/tests/tests.h
 @@ -29,6 +29,7 @@ int test__pmu(void);
  int test__attr(void);
  int test__dso_data(void);
  int test__dso_data_cache(void);
 +int test__dso_data_reopen(void);
  int test__parse_events(void);
  int test__hists_link(void);
  int test__python_use(void);
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/