On 2017/11/1 20:00, Namhyung Kim wrote:
On Wed, Nov 01, 2017 at 06:32:50PM +0800, Wangnan (F) wrote:

On 2017/11/1 17:49, Namhyung Kim wrote:
Hi,

On Wed, Nov 01, 2017 at 05:53:26AM +0000, Wang Nan wrote:
perf record backward recording doesn't work as we expected: it never
overwrite when ring buffer full.

Test:

(Run a busy printing python task background like this:

   while True:
       print 123

send SIGUSR2 to perf to capture snapshot.)
[SNIP]

Signed-off-by: Wang Nan <wangn...@huawei.com>
---
   tools/perf/util/evlist.c | 8 +++++++-
   1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c6c891e..4c5daba 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -799,22 +799,28 @@ perf_evlist__should_poll(struct perf_evlist *evlist 
__maybe_unused,
   }
   static int perf_evlist__mmap_per_evsel(struct perf_evlist *evlist, int idx,
-                                      struct mmap_params *mp, int cpu_idx,
+                                      struct mmap_params *_mp, int cpu_idx,
                                       int thread, int *_output, int 
*_output_backward)
   {
        struct perf_evsel *evsel;
        int revent;
        int evlist_cpu = cpu_map__cpu(evlist->cpus, cpu_idx);
+       struct mmap_params *mp;
        evlist__for_each_entry(evlist, evsel) {
                struct perf_mmap *maps = evlist->mmap;
+               struct mmap_params rdonly_mp;
                int *output = _output;
                int fd;
                int cpu;
+               mp = _mp;
                if (evsel->attr.write_backward) {
                        output = _output_backward;
                        maps = evlist->backward_mmap;
+                       rdonly_mp = *_mp;
+                       rdonly_mp.prot &= ~PROT_WRITE;
+                       mp = &rdonly_mp;
                        if (!maps) {
                                maps = perf_evlist__alloc_mmap(evlist);
--
What about this instead (not tested)?


diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index c6c891e154a6..27ebe355e794 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -838,6 +838,11 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist 
*evlist, int idx,
                  if (*output == -1) {
                          *output = fd;
+                       if (evsel->attr.write_backward)
+                               mp->prot = PROT_READ;
+                       else
+                               mp->prot = PROT_READ | PROT_WRITE;
+
If evlist->overwrite is true, PROT_WRITE should be unset even if
write_backward is
not set. If you want to delay the setting of mp->prot, you need to consider
both evlist->overwrite and evsel->attr.write_backward.
I thought evsel->attr.write_backward should be set when
evlist->overwrite is set.  Do you mean following case?

   perf record --overwrite -e 'cycles/no-overwrite/'


No. evlist->overwrite is unrelated to '--overwrite'. This is why I
said the concept of 'overwrite' and 'backward' is ambiguous.

perf record never sets 'evlist->overwrite'. What '--overwrite' actually
does is setting write_backward. Some testcases needs overwrite evlist.

Thank you.

Reply via email to