All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jin, Yao" <yao.jin@linux.intel.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
	mingo@redhat.com, alexander.shishkin@linux.intel.com,
	Linux-kernel@vger.kernel.org, ak@linux.intel.com,
	kan.liang@intel.com, yao.jin@intel.com
Subject: Re: [PATCH v2] perf pmu: Validate raw event with sysfs exported format bits
Date: Mon, 8 Mar 2021 11:12:25 +0800	[thread overview]
Message-ID: <4677dda8-6a3b-1908-775e-3510091abda5@linux.intel.com> (raw)
In-Reply-To: <YEPUJze6AUpNb8nr@krava>

Hi Jiri,

On 3/7/2021 3:12 AM, Jiri Olsa wrote:
> actualy we do have bitmap_scnprintf, which is already doing the
> printing in a nice way.. we could use it like below, only compile
> tested
> 
> jirka
> 
> 
> ---
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 31e975b75766..37ca5eee2667 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1818,7 +1818,7 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
>   {
>   	struct perf_pmu_format *format;
>   	__u64 masks = 0, bits;
> -	struct strbuf buf = STRBUF_INIT;
> +	char buf[50];
>   	unsigned int i;
>   
>   	list_for_each_entry(format, &pmu->format, list)	{
> @@ -1836,25 +1836,15 @@ void perf_pmu__warn_invalid_config(struct perf_pmu *pmu, __u64 config,
>   	 * Kernel doesn't export any valid format bits.
>   	 */
>   	if (masks == 0)
> -		goto out;
> +		return;
>   
>   	bits = config & ~masks;
>   	if (bits == 0)
> -		goto out;
> -
> -	for_each_set_bit(i, (unsigned long *)&bits, sizeof(bits) * 8)
> -		strbuf_addf(&buf, " %d", i);
> +		return;
>   
> -	if (name) {
> -		pr_warning("WARNING: event '%s' not valid (bits%s of config "
> -			   "'%llx' not supported by kernel)!\n",
> -			   name, buf.buf, config);
> -	} else {
> -		pr_warning("WARNING: event not valid (bits%s of config "
> -			   "'%llx' not supported by kernel)!\n",
> -			   buf.buf, config);
> -	}
> +	bitmap_scnprintf((unsigned long *)&bits, sizeof(bits) * 8, buf, sizeof(buf));
>   
> -out:
> -	strbuf_release(&buf);
> +	pr_warning("WARNING: event '%s' not valid (bits%s of config "
> +		   "'%llx' not supported by kernel)!\n",
> +		   name ?: "N/A", buf, config);
>   }

The bitmap_scnprintf works fine. Yes, it's really better than original 'strbuf' way, thanks!

BTW, I added "Co-developed-by: Jiri Olsa <jolsa@redhat.com>" in v3, but the patch checking shows the 
warning:

WARNING: Co-developed-by and Signed-off-by: name/email do not match
#76:
Co-developed-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Jin Yao <yao.jin@linux.intel.com>

I'm not sure if this warning is important or not, but anyway I add the line to v3. Thanks for your 
help!

Jin Yao

      reply	other threads:[~2021-03-08  3:13 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-05  0:56 [PATCH v2] perf pmu: Validate raw event with sysfs exported format bits Jin Yao
2021-03-06 19:12 ` Jiri Olsa
2021-03-08  3:12   ` Jin, Yao [this message]

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=4677dda8-6a3b-1908-775e-3510091abda5@linux.intel.com \
    --to=yao.jin@linux.intel.com \
    --cc=Linux-kernel@vger.kernel.org \
    --cc=acme@kernel.org \
    --cc=ak@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@kernel.org \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@intel.com \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=yao.jin@intel.com \
    /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.