lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aRwXKgv7iUjfvny_@google.com>
Date: Mon, 17 Nov 2025 22:50:18 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Ian Rogers <irogers@...gle.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	Jiri Olsa <jolsa@...nel.org>,
	Adrian Hunter <adrian.hunter@...el.com>,
	"Dr. David Alan Gilbert" <linux@...blig.org>,
	Yang Li <yang.lee@...ux.alibaba.com>,
	James Clark <james.clark@...aro.org>,
	Thomas Falcon <thomas.falcon@...el.com>,
	Thomas Richter <tmricht@...ux.ibm.com>,
	linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
	Andi Kleen <ak@...ux.intel.com>,
	Dapeng Mi <dapeng1.mi@...ux.intel.com>
Subject: Re: [PATCH v4 10/10] perf stat: Add no-affinity flag

On Mon, Nov 17, 2025 at 08:32:20PM -0800, Ian Rogers wrote:
> On Mon, Nov 17, 2025 at 6:41 PM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > On Thu, Nov 13, 2025 at 10:05:16AM -0800, Ian Rogers wrote:
> > > Add flag that disables affinity behavior. Using sched_setaffinity to
> > > place a perf thread on a CPU can avoid certain interprocessor
> > > interrupts but may introduce a delay due to the scheduling,
> > > particularly on loaded machines. Add a command line option to disable
> > > the behavior. This behavior is less present in other tools like `perf
> > > record`, as it uses a ring buffer and doesn't make repeated system
> > > calls.
> > >
> > > Signed-off-by: Ian Rogers <irogers@...gle.com>
> > > ---
> > >  tools/perf/Documentation/perf-stat.txt | 4 ++++
> > >  tools/perf/builtin-stat.c              | 6 ++++++
> > >  tools/perf/util/evlist.c               | 2 +-
> > >  tools/perf/util/evlist.h               | 1 +
> > >  4 files changed, 12 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
> > > index 1a766d4a2233..1ffb510606af 100644
> > > --- a/tools/perf/Documentation/perf-stat.txt
> > > +++ b/tools/perf/Documentation/perf-stat.txt
> > > @@ -382,6 +382,10 @@ color the metric's computed value.
> > >  Don't print output, warnings or messages. This is useful with perf stat
> > >  record below to only write data to the perf.data file.
> > >
> > > +--no-affinity::
> > > +Don't change scheduler affinities when iterating over CPUs. Disables
> > > +an optimization aimed at minimizing interprocessor interrupts.
> > > +
> > >  STAT RECORD
> > >  -----------
> > >  Stores stat data into perf data file.
> > > diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> > > index aec93b91fd11..fa42b08bd1df 100644
> > > --- a/tools/perf/builtin-stat.c
> > > +++ b/tools/perf/builtin-stat.c
> > > @@ -2415,6 +2415,7 @@ static int parse_tpebs_mode(const struct option *opt, const char *str,
> > >  int cmd_stat(int argc, const char **argv)
> > >  {
> > >       struct opt_aggr_mode opt_mode = {};
> > > +     bool no_affinity = false;
> > >       struct option stat_options[] = {
> > >               OPT_BOOLEAN('T', "transaction", &transaction_run,
> > >                       "hardware transaction statistics"),
> > > @@ -2543,6 +2544,8 @@ int cmd_stat(int argc, const char **argv)
> > >                       "don't print 'summary' for CSV summary output"),
> > >               OPT_BOOLEAN(0, "quiet", &quiet,
> > >                       "don't print any output, messages or warnings (useful with record)"),
> > > +             OPT_BOOLEAN(0, "no-affinity", &no_affinity,
> > > +                     "don't allow affinity optimizations aimed at reducing IPIs"),
> >
> > I know you want to add an option to disable the behaivor, but I think
> > it'd better to have a positive option like just '--affinity'.  Then we
> > will have '--no-affinity' for free. :)  The current form will allow
> > '--no-no-affinity'.
> >
> > Then the variable also can be 'enable_affinity' or so.
> >
> > You can mention --no-affinity in the help message and the man page
> > document so that users can discover the intention.
> 
> I was trying to keep the code the same as the flag. I don't want to
> have an affinity flag you need to set to true in say evlist, as that
> will need updating in all users of the evlist or become a behavioral
> change. We can have the evlist with no_affinity and invert the flag,
> it just looks awkward imo.

You can make the default to true and just update the users that want
different behaviors.

Thanks,
Namhyung


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ