From: Jayachandran C <jnair@caviumnetworks.com>
To: Will Deacon <will.deacon@arm.com>
Cc: "Pinski, Andrew" <Andrew.Pinski@cavium.com>,
"Jayachandran C." <c.jayachandran@gmail.com>,
Ganapatrao Kulkarni <gpkulkarni@gmail.com>,
Mark Rutland <mark.rutland@arm.com>,
Catalin Marinas <catalin.marinas@arm.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"acme@kernel.org" <acme@kernel.org>,
"alexander.shishkin@linux.intel.com"
<alexander.shishkin@linux.intel.com>,
"peterz@infradead.org" <peterz@infradead.org>,
Ingo Molnar <mingo@redhat.com>,
"Nair, Jayachandran" <Jayachandran.Nair@cavium.com>,
"Kulkarni, Ganapatrao" <Ganapatrao.Kulkarni@cavium.com>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP
Date: Wed, 26 Apr 2017 13:41:42 +0000 [thread overview]
Message-ID: <20170426134141.GA6417@localhost> (raw)
In-Reply-To: <20170426101021.GF21744@arm.com>
On Wed, Apr 26, 2017 at 11:10:21AM +0100, Will Deacon wrote:
> On Wed, Apr 26, 2017 at 07:22:46AM +0000, Pinski, Andrew wrote:
> > On 4/25/2017 11:53 PM, Jayachandran C. wrote:
> > > On Tue, Apr 25, 2017 at 10:23 PM, Will Deacon <will.deacon@arm.com> wrote:
> > >> On Tue, Apr 25, 2017 at 09:13:40AM +0530, Ganapatrao Kulkarni wrote:
> > >>> On Mon, Apr 24, 2017 at 9:15 PM, Will Deacon <will.deacon@arm.com> wrote:
> > >>>> On Thu, Apr 20, 2017 at 02:56:50PM +0530, Ganapatrao Kulkarni wrote:
> > >>>>> OK, if you are ok with sysfs part, i can send next version with that
> > >>>>> change only?.
> > >>>> I think the sysfs part is still a little dodgy, since you still expose the
> > >>>> "exclude_hv" file with a value of 0 when not running at EL2, which would
> > >>>> imply that exclude_hv is forced to zero. I don't think that's correct.
> > >>> okay, i can make exclude_hv visible only when kernel booted in EL2.
> > >>> is it ok to have empty directory "attr" when kernel booted to EL1?
> > >>> attr can be place holder for any other miscellaneous attributes, that
> > >>> can be added in future.
> > >> Sounds good to me, although I'll seek comment from the other perf folks
> > >> before merging anything with ABI implications.
> > > Do you really think this is the solution given:
> > > - this is an arm64 specific sysfs interface that is tied to the perf API
>
> That's why I want feedback from others. The intention would be that this can
> be used by other PMUs as well, since it's not uncommon that parts of the
> sizeable perf_event_attr structure are not used by a given PMU.
>
> > > - the perf API documentation has to be updated for this
>
> So? If having to update documentation means we shouldn't change the kernel,
> then we may as well all find new jobs.
>
> > > - All the applications that use the perf API have to be modified to
> > > check this sysfs interface
> > > - If the application fails to do so, a very narrow corner case
> > > (exclude_hv != exclude_kernel and VHE enabled) fails.
>
> See below, but apparently people care about it.
>
> > > Any application that really cares can already do see if exclude_hv !=
> > > exclude_kernel case works by calling perf_open_event() with those
> > > options and checking the return value.
>
> That's a good point: there is *something* userspace can do, although that
> would be arm64-specific and doesn't really help with the state-space
> explosion you get with combinations of invalid/unused perf_event_attr
> fields.
>
> > An example of an application which needs to changed is HHVM. Currently
> > it sets exclude_hv to true but exclude_kernel to false as it does not
> > care about the hypervisor associated perf events associated with the
> > code, only the kernel and userspace associated evnts.
> > Yes we could submit a patch to use the sysfs interface to check but it
> > would look funny and the facebook folks might reject the patch as it is
> > ARM64 specific in generic code. Note this is how all of this discussion
> > started was HHVM's call to perf_open_event was failing.
>
> Hmm, if you're saying that HHVM won't be changed to use the sysfs stuff,
> then why are we bothering?
>
> Not sure where this leaves us.
If my understanding is correct, the sysfs suggestion above is going to
add API complexity without solving the issue. Ignoring the exclude_hv if
it cannot be honored would be a better solution.
If that is not acceptable (which seems to be the case - but I do not
see a reason for that), I think the better option for the application
is to check if the platform supports the mode exclusion it wants by
using the perf_event_open API itself.
Thanks,
JC.
next prev parent reply other threads:[~2017-04-26 13:42 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-19 17:44 [PATCH v2] arm64: perf: Use only exclude_kernel attribute when kernel is running in HYP Ganapatrao Kulkarni
2017-04-20 8:49 ` Mark Rutland
2017-04-20 9:26 ` Ganapatrao Kulkarni
2017-04-24 15:45 ` Will Deacon
2017-04-25 3:43 ` Ganapatrao Kulkarni
2017-04-25 16:53 ` Will Deacon
2017-04-26 6:53 ` Jayachandran C.
[not found] ` <CY4PR07MB3415C0CEBB9D3DF0C249DC6793110@CY4PR07MB3415.namprd07.prod.outlook.com>
2017-04-26 10:10 ` Will Deacon
2017-04-26 13:41 ` Jayachandran C [this message]
2017-04-27 17:37 ` Will Deacon
2017-04-28 13:46 ` Jayachandran C
2017-04-28 16:38 ` Will Deacon
2017-05-01 16:10 ` Jayachandran C
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=20170426134141.GA6417@localhost \
--to=jnair@caviumnetworks.com \
--cc=Andrew.Pinski@cavium.com \
--cc=Ganapatrao.Kulkarni@cavium.com \
--cc=Jayachandran.Nair@cavium.com \
--cc=acme@kernel.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=c.jayachandran@gmail.com \
--cc=catalin.marinas@arm.com \
--cc=gpkulkarni@gmail.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=will.deacon@arm.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 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).