Re: [PATCH v2 1/5] perf record: Set PERF_RECORD_PERIOD if attr->freq is set.

Athira Rajeev
2020-07-29T15:12:20Z

> On 28-Jul-2020, at 9:33 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
>
> Em Tue, Jul 28, 2020 at 05:43:47PM +0200, Jiri Olsa escreveu:
>> On Tue, Jul 28, 2020 at 01:57:30AM -0700, Ian Rogers wrote:
>>> From: David Sharp <dhsharp@google.com>
>>>
>>> evsel__config() would only set PERF_RECORD_PERIOD if it set attr->freq
>>> from perf record options. When it is set by libpfm events, it would not
>>> get set. This changes evsel__config to see if attr->freq is set outside of
>>> whether or not it changes attr->freq itself.
>>>
>>> Signed-off-by: David Sharp <dhsharp@google.com>
>>> Signed-off-by: Ian Rogers <irogers@google.com>
>>
>> Acked-by: Jiri Olsa <jolsa@redhat.com>
>
> So, somebody else complained that its not PERF_RECORD_PERIOD (there is
> no such thing) that is being set, its PERF_SAMPLE_PERIOD.

Hi Arnaldo

Thanks for adding in that correction.

Athira
>
> Since you acked it I merged it now, with that correction,
>
> - Arnaldo
>
>> thanks,
>> jirka
>>
>>> ---
>>> tools/perf/util/evsel.c | 7 ++++++-
>>> 1 file changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
>>> index ef802f6d40c1..811f538f7d77 100644
>>> --- a/tools/perf/util/evsel.c
>>> +++ b/tools/perf/util/evsel.c
>>> @@ -979,13 +979,18 @@ void evsel__config(struct evsel *evsel, struct record_opts *opts,
>>> if (!attr->sample_period || (opts->user_freq != UINT_MAX ||
>>> opts->user_interval != ULLONG_MAX)) {
>>> if (opts->freq) {
>>> - evsel__set_sample_bit(evsel, PERIOD);
>>> attr->freq = 1;
>>> attr->sample_freq = opts->freq;
>>> } else {
>>> attr->sample_period = opts->default_interval;
>>> }
>>> }
>>> + /*
>>> + * If attr->freq was set (here or earlier), ask for period
>>> + * to be sampled.
>>> + */
>>> + if (attr->freq)
>>> + evsel__set_sample_bit(evsel, PERIOD);
>>>
>>> if (opts->no_samples)
>>> attr->sample_freq = 0;
>>> --
>>> 2.28.0.163.g6104cc2f0b6-goog
>>>
>>
>
> --
>
> - Arnaldo