linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@kernel.org>
To: Rob Herring <robh@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Will Deacon <will@kernel.org>,
	Kan Liang <kan.liang@linux.intel.com>
Cc: 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>,
	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: Thu, 12 Aug 2021 09:50:39 -0700	[thread overview]
Message-ID: <d720903c-926e-f57a-0862-4e5d76db763a@kernel.org> (raw)
In-Reply-To: <20210728230230.1911468-3-robh@kernel.org>

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.

If you intend to have an entire mm have access to RDPMC if an event is
mapped, then surely access needs to be context switched for the whole
mm.  If you intend to only have the thread to which the event is bound
have access, then the only reason I see to use IPIs is to revoke access
on munmap from the wrong thread.  But even that latter case could be
handled with a more targeted approach, not a broadcast to all of mm_cpumask.

Can you clarify what the overall intent is and what this particular
patch is trying to do?

> 
>  	if (atomic_dec_and_test(&mm->context.perf_rdpmc_allowed))
> -		on_each_cpu_mask(mm_cpumask(mm), cr4_update_pce, NULL, 1);
> +		on_each_cpu_mask(mm_cpumask(mm), x86_pmu_set_user_access_ipi, NULL, 1);

Here you do something for the whole mm.

> -		on_each_cpu(cr4_update_pce, NULL, 1);
> +		on_each_cpu(x86_pmu_set_user_access_ipi, NULL, 1);

Here too.

>  void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  			struct task_struct *tsk)
>  {
> @@ -581,10 +556,8 @@ void switch_mm_irqs_off(struct mm_struct *prev, struct mm_struct *next,
>  	this_cpu_write(cpu_tlbstate.loaded_mm, next);
>  	this_cpu_write(cpu_tlbstate.loaded_mm_asid, new_asid);
> 
> -	if (next != real_prev) {
> -		cr4_update_pce_mm(next);
> +	if (next != real_prev)
>  		switch_ldt(real_prev, next);
> -	}
>  }

But if you remove this, then what handles context switching?

> 
>  /*
> --
> 2.27.0
> 


  reply	other threads:[~2021-08-12 16:50 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 [this message]
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
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=d720903c-926e-f57a-0862-4e5d76db763a@kernel.org \
    --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).