All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>,
	bpf@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH] perf: record: Clear BPF headers if not needed
Date: Fri, 3 Jul 2026 17:14:29 -0700	[thread overview]
Message-ID: <akhQZTF_yveg840q@google.com> (raw)
In-Reply-To: <20260701143148.0B4811F000E9@smtp.kernel.org>

On Wed, Jul 01, 2026 at 02:31:47PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch conditionally clears BPF header features based on `rec->opts.no_bpf_event` too early in `record__init_features()`, failing to fix the spurious error messages for default and dynamically evaluated cases when `perf` is compiled without `libbpf`.
> --
> 
> commit 9515d1b27d6dcc9884718d4505a8cf399d665c9e
> Author: Mayuresh Chitale <mayuresh.chitale@oss.qualcomm.com>
> 
> perf: record: Clear BPF headers if not needed
> 
> This patch clears the HEADER_BPF_PROG_INFO and HEADER_BPF_BTF bits from the
> file header feature bitmask when the '--no-bpf-event' flag is passed. This
> aims to prevent spurious error messages about missing libbpf support when
> writing perf data.
> 
> > diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> > index e915390556752..692ab7138d2c3 100644
> > --- a/tools/perf/builtin-record.c
> > +++ b/tools/perf/builtin-record.c
> > @@ -1829,6 +1829,11 @@ static void record__init_features(struct record *rec)
> >  	if (!record__comp_enabled(rec))
> >  		perf_header__clear_feat(&session->header, HEADER_COMPRESSED);
> >  
> > +	if (rec->opts.no_bpf_event) {
> > +		perf_header__clear_feat(&session->header, HEADER_BPF_PROG_INFO);
> > +		perf_header__clear_feat(&session->header, HEADER_BPF_BTF);
> > +	}
> > +
> 
> [Severity: Medium]
> Does this conditionally clear the features too early in the initialization
> process?
> 
> If the user does not explicitly pass the flag (the default case), or if the
> flag is dynamically updated to true later (e.g. if updated after checking
> !evlist__needs_bpf_sb_event(rec->evlist)), this condition will evaluate to
> false.

Probably we can move this check before record__init_features().  It
seems there's nothing can change the evlist between the two points.

Thanks,
Namhyung

> 
> Will this leave the spurious errors unresolved for default and dynamically
> evaluated cases when perf is compiled without libbpf?
> 
> >  	perf_header__clear_feat(&session->header, HEADER_STAT);
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260701141347.4098670-1-mayuresh.chitale@oss.qualcomm.com?part=1

      reply	other threads:[~2026-07-04  0:14 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-01 14:13 [PATCH] perf: record: Clear BPF headers if not needed Mayuresh Chitale
2026-07-01 14:31 ` sashiko-bot
2026-07-04  0:14   ` Namhyung Kim [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=akhQZTF_yveg840q@google.com \
    --to=namhyung@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mayuresh.chitale@oss.qualcomm.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /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.