linux-kernel - Re: [PATCH 03/10] perf tool: new iterfaces to read event from ring buffer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 11 Oct 2017 03:22:30 +0800
From:   "Wangnan (F)" <wangnan0@...wei.com>
To:     Arnaldo Carvalho de Melo <acme@...nel.org>
CC:     "Liang, Kan" <kan.liang@...el.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jolsa@...nel.org" <jolsa@...nel.org>,
        "hekuang@...wei.com" <hekuang@...wei.com>,
        "namhyung@...nel.org" <namhyung@...nel.org>,
        "alexander.shishkin@...ux.intel.com" 
        <alexander.shishkin@...ux.intel.com>,
        "Hunter, Adrian" <adrian.hunter@...el.com>,
        "ak@...ux.intel.com" <ak@...ux.intel.com>
Subject: Re: [PATCH 03/10] perf tool: new iterfaces to read event from ring
 buffer



On 2017/10/11 3:17, Arnaldo Carvalho de Melo wrote:
> Em Wed, Oct 11, 2017 at 03:10:37AM +0800, Wangnan (F) escreveu:
>>
>> On 2017/10/11 3:00, Arnaldo Carvalho de Melo wrote:
>>> Em Tue, Oct 10, 2017 at 03:36:28PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>> Em Tue, Oct 10, 2017 at 03:34:55PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>>> Em Tue, Oct 10, 2017 at 06:28:18PM +0000, Liang, Kan escreveu:
>>>>>>> Em Tue, Oct 10, 2017 at 10:20:16AM -0700, kan.liang@...el.com escreveu:
>>>>>>>> From: Kan Liang <kan.liang@...el.com>
>>>>>>>>
>>>>>>>> The perf_evlist__mmap_read only support forward mode. It needs a
>>>>>>>> common function to support both forward and backward mode.
>>>>>>>> The perf_evlist__mmap_read_backward is buggy.
>>>>>>> So, what is the bug? You state that it is buggy, but don't spell out the bug,
>>>>>>> please do so.
>>>>>>>
>>>>>> union perf_event *perf_evlist__mmap_read_backward(struct perf_evlist *evlist, int idx)
>>>>>> {
>>>>>> 	struct perf_mmap *md = &evlist->mmap[idx];  <--- it should be backward_mmap
>>>>>>
>>>>>>> If it fixes an existing bug, then it should go separate from this patchkit, right?
>>>>>> There is no one use perf_evlist__mmap_read_backward. So it doesn't trigger any issue.
>>>>> There is no one at the end of your patchkit? Or no user _right now_? If
>>>>> there is a user now, lemme see... yeah, no user right now, so _that_ is
>>>>> yet another bug, i.e. it should be used, no? If this is just a left
>>>>> over, then we should just throw it away, now, its a cleanup.
>>>> Wang, can you take a look at these two issues?
>>> So it looks leftover that should've been removed by the following cset, right Wang?
>>> commit a0c6f451f90204847ce5f91c3268d83a76bde1b6
>>> Author: Wang Nan <wangnan0@...wei.com>
>>> Date:   Thu Jul 14 08:34:41 2016 +0000
>>>       perf evlist: Drop evlist->backward
>>>       Now there's no real user of evlist->backward. Drop it. We are going to
>>>       use evlist->backward_mmap as a container for backward ring buffer.
>   
>> Yes, it should be removed, but then there will be no corresponding
>> function to perf_evlist__mmap_read(), which read an record from forward
>> ring buffer.
>   
>> I think Kan wants to become the first user of this function because
>> he is trying to make 'perf top' utilizing backward ring buffer. It needs
>> perf_evlist__mmap_read_backward(), and he triggers the bug use his
>> unpublished patch set.
>   
>> I think we can remove it now, let Kan fix and add it back in his 'perf top'
>> patch set.
> Well, if there will be a user, perhaps we should fix it, as it seems
> interesting to have now for, as you said, a counterpart for the forward
> ring buffer, and one that we have plans for using soon, right?

Right if I understand Kan's patch 00/10 correctly. He said:

...
    But perf top need to switch to overwrite backward mode for good
    performance.
...

Thank you.