Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1379981

Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping

From Arnaldo Carvalho de Melo <acme@kernel.org>
Newsgroups linux.kernel
Subject Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping
Date 2016-04-15 18:30 +0200
Message-ID <roeN4-5WB-9@gated-at.bofh.it> (permalink)
References <rnols-6Jw-11@gated-at.bofh.it> <ro9u2-1AH-19@gated-at.bofh.it> <ro9u2-1AH-17@gated-at.bofh.it> <roaq6-2jC-9@gated-at.bofh.it> <robFv-3uA-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Em Fri, Apr 15, 2016 at 10:09:32AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Fri, Apr 15, 2016 at 07:40:44PM +0800, Wangnan (F) escreveu:
> > On 2016/4/15 18:45, Wangnan (F) wrote:
> > >On 2016/4/15 18:40, Jiri Olsa wrote:
> > >>On Wed, Apr 13, 2016 at 08:21:03AM +0000, Wang Nan wrote:
> > 
> > [SNIP]
> > 
> > >>I did not get 3/10 patch and the patchset did not apply cleanly,
> > >>git am failed.. would you have it in a branch somewhere?
> > >
> > >Sorry, you are not in the CC list. 'git send-email' failed to extract your
> > >email address from the Acked-by tag.
> > >
> > >I'll inform you after I putting them into a git branch. Please wait.
> > >
> > >Thank you.
> > 
> > I just realized Arnaldo has already collected these patches set into
> > his perf/core. Please see it in his git tree [1]. You can also have a look
> > at my git tree [2] if you want :)
> 
> I haven't pushed them to Ingo yet, so I can fix up things if Jiri has
> any fixes or other requests,

I moved those patches to a separate branch, perf/switch_output, till we get a
bit more review, I think I was too fast on tentatively processing this patchset
and have some questions, for instance, this part I thin really confusing, in
the main record loop:

       	switch_output_enable();
        for (;;) {
                unsigned long long hits = rec->samples;

                if (record__mmap_read_all(rec) < 0) {
                        auxtrace_snapshot_disable();
                        err = -1;
                        goto out_child;
                }
<SNIP>
                if (switch_output_is_disabled()) {
                        switch_output_enable();

                        if (!quiet)
                                fprintf(stderr, "[ perf record: dump data: Woken up %ld times ]\n",
                                        waking);
                        waking = 0;
                        fd = record__switch_output(rec, false);
                        if (fd < 0) {
                                pr_err("Failed to switch to new file\n");
                                err = fd;
                                goto out_child;
                        }
                }
<SNIP>
	}

That switch_output_enable() one we can't get to because it is part of that
trigger_ thing, so just by looking here we think switch_output is being enabled
unconditionally, when in fact it will check if it is "OFF" and if so, will not
"enable", then when we see switch_output_is_disabled() the question will return
false if it is "OFF", but what we read is "hey, this is not disabled, so it
must be enabled, no? Confusing :-\

Perhaps we should have multiple record loops, one really simple, the original
one, one for auxtrace, that, from what we've discussed so far, doesn't lok will
be supported together with output switch, and one for output switch?

Would be something like:

	if (switch_output)
		err = record__switch_output_read_events()
	else if (auxtrace)
		err = record__auxtrace_read_events()
	else
		err = record__read_events();

And then each of these loops don't need to have all those branches per mmap_read.

- Arnaldo
 
> - Arnaldo
>  
> > [1] https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/core&id=c0bdc1c461dd5b2e492c0746708a3e0da6b13515
> > [2] https://git.kernel.org/cgit/linux/kernel/git/pi3orama/linux.git/log/?h=perf/overwrite

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
  [PATCH 08/10] perf record: Re-synthesize tracking events after output switching Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
  [PATCH 04/10] perf record: Add '--timestamp-filename' option to append timestamp to output filename Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
    [tip:perf/core] perf record: Add '--timestamp-filename' option to  append timestamp to output file name tip-bot for Wang Nan <tipbot@zytor.com> - 2016-04-14 15:40 +0200
  [PATCH 10/10] perf core: Add backward attribute to perf event Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
  [PATCH 02/10] perf tools: Add perf_data_file__switch() helper Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
    [tip:perf/core] perf data: Add perf_data_file__switch() helper tip-bot for Wang Nan <tipbot@zytor.com> - 2016-04-14 15:40 +0200
    Re: [PATCH 02/10] perf tools: Add perf_data_file__switch() helper Jiri Olsa <jolsa@redhat.com> - 2016-04-15 13:00 +0200
      Re: [PATCH 02/10] perf tools: Add perf_data_file__switch() helper Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-15 18:10 +0200
  [PATCH 09/10] perf record: Generate tracking events for process forked by perf Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
  [PATCH 05/10] perf record: Split output into multiple files via '--switch-output' Wang Nan <wangnan0@huawei.com> - 2016-04-13 10:30 +0200
  Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-13 19:20 +0200
  Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping "Wangnan (F)" <wangnan0@huawei.com> - 2016-04-15 12:50 +0200
    Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping "Wangnan (F)" <wangnan0@huawei.com> - 2016-04-15 13:50 +0200
      Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-15 15:10 +0200
        Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-04-15 18:30 +0200
          Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping "Wangnan (F)" <wangnan0@huawei.com> - 2016-04-15 18:50 +0200
            Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping "Wangnan (F)" <wangnan0@huawei.com> - 2016-04-15 20:00 +0200
  Re: [PATCH 00/10] perf tools: Use SIGUSR2 control data dumpping Jiri Olsa <jolsa@redhat.com> - 2016-04-15 12:50 +0200

csiph-web