Re: [PATCH v2 0/5] perf, bts: Fallout from the fuzzer for perf/urgent

[Alexander Shishkin]

Ingo Molnar <mingo@xxxxxxxxxx> writes:

> * Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> wrote:
>
>> Ingo Molnar <mingo@xxxxxxxxxx> writes:
>> 
>> > * Alexander Shishkin <alexander.shishkin@xxxxxxxxxxxxxxx> wrote:
>> >
>> >> Hi,
>> >> 
>> >> There were more bugs since the previous version, plus the BTS barriers got 
>> >> fixed. With these patches, my testcase keeps running and no spurious NMI 
>> >> warnings pop up any more.
>> >
>> > Could you please also run the fuzzer that Vince uses, does it now pass on hardware 
>> > you have access to?
>> 
>> Sure. And yes, I did catch a warning, which calls for one more patch
>> (below). Also one unrelated thing in PEBS that Peter fixed.
>> 
>> > I'd like to make "passes the fuzzer" a standard requirement before new changes are 
>> > accepted to perf core.
>> 
>> Let's make it so.
>> 
>> For the sake of consistency, this one needs to go before 3/5. I'll
>> re-send the whole series, though, if need be. I've got 2 perf_fuzzers
>> running on this meanwhile.
>
> Yeah, please re-send it - and please also Vince's Reported-by tag to all commits 
> that would explain failures that Vince reported.
>
> Also, please document how much and what type of fuzzer testing the series got: 
> fuzzer version, time it ran and (rough) hardware it ran on would be useful. (That 
> way we can look back later on whether there was any fuzzer testing on AMD systems 
> for example, which you might not be able to perform.)

Not sure if run time is useful with the fuzzer, but otherwise seems
reasonable. How do we put this stuff into a commit message, though?

Perf-Fuzzer-ID: d18d23aae6

?

> It would also be very, very nice to also add a Documentation/perf/testing.txt step 
> by step ELI5 style document that explains how to set up and run the fuzzer!

Actually, I'd like that too as I'm not sure I'm doing it 100% right (had
to patch it some time ago to stop it from segfaulting).

>> + * intel_bts events don't coexist with intel pmu's BTS events because of
>> + * x86_add_exclusive(x86_lbr_exclusive_lbr); there's no need to keep them
>> + * disabled around intel pmu's event batching etc, only inside the PMI handler.
>
> Pet peeve nit: please capitalize 'PMU' correctly and consistently (upper case). 
> This paragraph has both variants: "PMU" and "pmu" which is the worst variant 
> really.

Sure.

Regards,
--
Alex