linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Andy Lutomirski" <luto@kernel.org>
To: "Rob Herring" <robh@kernel.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>,
	"Mark Rutland" <mark.rutland@arm.com>,
	"Will Deacon" <will@kernel.org>,
	"Kan Liang" <kan.liang@linux.intel.com>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>,
	"Ingo Molnar" <mingo@redhat.com>,
	"Arnaldo Carvalho de Melo" <acme@kernel.org>,
	"Alexander Shishkin" <alexander.shishkin@linux.intel.com>,
	"Jiri Olsa" <jolsa@redhat.com>,
	"Namhyung Kim" <namhyung@kernel.org>,
	"Thomas Gleixner" <tglx@linutronix.de>,
	"Borislav Petkov" <bp@alien8.de>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	"Dave Hansen" <dave.hansen@linux.intel.com>,
	linux-perf-users@vger.kernel.org
Subject: Re: [RFC 2/3] perf/x86: Control RDPMC access from .enable() hook
Date: Fri, 27 Aug 2021 14:10:42 -0700	[thread overview]
Message-ID: <43b3a838-da8a-4733-9832-f3d5f990ec13@www.fastmail.com> (raw)
In-Reply-To: <CAL_JsqKpT93W6nBj68DykEJzjFYOPG=8PGShsh2QZVzHq5N3fQ@mail.gmail.com>



On Thu, Aug 26, 2021, at 12:09 PM, Rob Herring wrote:
> On Thu, Aug 26, 2021 at 1:13 PM Andy Lutomirski <luto@kernel.org> wrote:
> >
> >
> >
> > On Thu, Aug 12, 2021, at 11:16 AM, Rob Herring wrote:
> > > On Thu, Aug 12, 2021 at 11:50 AM Andy Lutomirski <luto@kernel.org> wrote:
> > > >
> > > > On 7/28/21 4:02 PM, Rob Herring wrote:
> > > > > Rather than controlling RDPMC access behind the scenes from switch_mm(),
> > > > > move RDPMC access controls to the PMU .enable() hook. The .enable() hook
> > > > > is called whenever the perf CPU or task context changes which is when
> > > > > the RDPMC access may need to change.
> > > > >
> > > > > This is the first step in moving the RDPMC state tracking out of the mm
> > > > > context to the perf context.
> > > >
> > > > Is this series supposed to be a user-visible change or not?  I'm confused.
> > >
> > > It should not be user-visible. Or at least not user-visible for what
> > > any user would notice. If an event is not part of the perf context on
> > > another thread sharing the mm, does that thread need rdpmc access? No
> > > access would be a user-visible change, but I struggle with how that's
> > > a useful scenario?
> > >
> >
> > This is what I mean by user-visible -- it changes semantics in a way that a user program could detect.  I'm not saying it's a problem, but I do think you need to document the new semantics.
> 
> After testing some scenarios and finding perf_event_tests[1], this
> series isn't going to work for x86 unless rdpmc is restricted to task
> events only or allowed to segfault on CPU events when read on the
> wrong CPU rather than just returning garbage. It's been discussed
> before here[2].
> 
> Ultimately, I'm just trying to define the behavior for arm64 where we
> don't have an existing ABI to maintain and don't have to recreate the
> mistakes of x86 rdpmc ABI. Tying the access to mmap is messy. As we
> explicitly request user access on perf_event_open(), I think it may be
> better to just enable access when the event's context is active and
> ignore mmap(). Maybe you have an opinion there since you added the
> mmap() part?

That makes sense to me. The mmap() part was always a giant kludge.

There is fundamentally a race, at least if rseq isn’t used: if you check that you’re on the right CPU, do RDPMC, and throw out the result if you were on the wrong CPU (determined by looking at the mmap), you still would very much prefer not to fault.

Maybe rseq or a vDSO helper is the right solution for ARM.

> 
> Rob
> 
> [1] https://github.com/deater/perf_event_tests
> [2] https://lore.kernel.org/lkml/alpine.DEB.2.21.1901101229010.3358@macbook-air/
> 

  reply	other threads:[~2021-08-27 21:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-28 23:02 [RFC 0/3] perf/x86: Rework RDPMC access handling Rob Herring
2021-07-28 23:02 ` [RFC 1/3] x86: perf: Move RDPMC event flag to a common definition Rob Herring
2021-07-28 23:02 ` [RFC 2/3] perf/x86: Control RDPMC access from .enable() hook Rob Herring
2021-08-12 16:50   ` Andy Lutomirski
2021-08-12 18:16     ` Rob Herring
2021-08-26 18:13       ` Andy Lutomirski
2021-08-26 19:09         ` Rob Herring
2021-08-27 21:10           ` Andy Lutomirski [this message]
2021-08-30  3:05             ` Vince Weaver
2021-08-30  8:51               ` Peter Zijlstra
2021-08-30 20:21                 ` Vince Weaver
2021-08-30 21:40                   ` Rob Herring
2021-08-30 20:58               ` Rob Herring
2021-07-28 23:02 ` [RFC 3/3] perf/x86: Call mmap event callbacks on event's CPU Rob Herring

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=43b3a838-da8a-4733-9832-f3d5f990ec13@www.fastmail.com \
    --to=luto@kernel.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=robh@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    --subject='Re: [RFC 2/3] perf/x86: Control RDPMC access from .enable() hook' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).