All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>, Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	LKML <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@linux.intel.com>, Ian Rogers <irogers@google.com>,
	Alexey Alexandrov <aalexand@google.com>
Subject: Re: [PATCH] perf record: Disallow -c and -F option at the same time
Date: Fri, 2 Apr 2021 10:37:02 -0300	[thread overview]
Message-ID: <YGcd/vz3KyOUReOh@kernel.org> (raw)
In-Reply-To: <20210402094020.28164-1-namhyung@kernel.org>

Em Fri, Apr 02, 2021 at 06:40:20PM +0900, Namhyung Kim escreveu:
> It's confusing which one is effective when the both options are given.
> The current code happens to use -c in this case but users might not be
> aware of it.  We can change it to complain about that instead of
> relying on the implicit priority.
> 
> Before:
>   $ perf record -c 111111 -F 99 true
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.031 MB perf.data (8 samples) ]
> 
>   $ perf evlist -F
>   cycles: sample_period=111111
> 
> After:
>   $ perf record -c 111111 -F 99 true
>   cannot set frequency and period at the same time
> 
> So this change can break existing usages, but I think it's rare to
> have both options and it'd be better changing them.

Humm, perhaps we can just make that an warning stating that -c is used
if both are specified?

$ perf record -c 111111 -F 99 true
Frequency and period can't be used the same time, -c 11111 will be used.

- Arnaldo
 
> Suggested-by: Alexey Alexandrov <aalexand@google.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/util/record.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index f99852d54b14..43e5b563dee8 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -157,9 +157,15 @@ static int get_max_rate(unsigned int *rate)
>  static int record_opts__config_freq(struct record_opts *opts)
>  {
>  	bool user_freq = opts->user_freq != UINT_MAX;
> +	bool user_interval = opts->user_interval != ULLONG_MAX;
>  	unsigned int max_rate;
>  
> -	if (opts->user_interval != ULLONG_MAX)
> +	if (user_interval && user_freq) {
> +		pr_err("cannot set frequency and period at the same time\n");
> +		return -1;
> +	}
> +
> +	if (user_interval)
>  		opts->default_interval = opts->user_interval;
>  	if (user_freq)
>  		opts->freq = opts->user_freq;
> -- 
> 2.31.0.208.g409f899ff0-goog
> 

-- 

- Arnaldo

  reply	other threads:[~2021-04-02 13:37 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-02  9:40 [PATCH] perf record: Disallow -c and -F option at the same time Namhyung Kim
2021-04-02 13:37 ` Arnaldo Carvalho de Melo [this message]
     [not found]   ` <CANtFmmo6Wq_u=D89hkfmS8yMtKhk+6pm_4YawpO-6MvS_4noaw@mail.gmail.com>
2021-04-03 17:13     ` Arnaldo Carvalho de Melo
     [not found]       ` <CANtFmmqeBPW7XHAvphw9YpGy74b3T-jfzg1op+9ZyU=YLDLjDg@mail.gmail.com>
2021-05-03 21:32         ` Namhyung Kim
2021-05-04 12:38           ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YGcd/vz3KyOUReOh@kernel.org \
    --to=acme@kernel.org \
    --cc=aalexand@google.com \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=irogers@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.