linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: nleeder@codeaurora.org (Leeder, Neil)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v9] perf: add qcom l2 cache perf events driver
Date: Mon, 6 Feb 2017 14:11:36 -0500	[thread overview]
Message-ID: <8d61d7fb-71d0-190e-26e3-a72e98cfb10d@codeaurora.org> (raw)
In-Reply-To: <20170206154814.GA4190@leverpostej>

Hi Mark,
Thanks for those comments - I'll add the fixes.

On 2/6/2017 10:48 AM, Mark Rutland wrote:
> I'm still concerned by this use of the filter_match callback, because it
> depends on the set of other active events, and can change as other
> events are scheduled in and out.
>
> When we schedule in two conflicting events A and B in order, B will fail
> its filter match. When we scheduled out A and B in order, B will succeed
> its filter match.
>
> The perf core does not expect this inconsistency, and this appears to
> break the timing update logic in event_sched_out(), when unconditionally
> called from ctx_sched_out() as part of perf_rotate_context().
>
> I would feel much happier if we dropped l2_cache_filter_match(), at
> least for the timebeing, and handled this as we do for other cases of
> intra-pmu resource contention.
>
> We can then consider the filter_match addition on its own at a later
> point.

So could this be detected in get_event_idx, the same way we handle 
counter resource contention? That would eliminate filter_match, and it's 
the same way its done in armv7 
(arch/arm/kernel/perf_event_v7.c:krait_pmu_get_event_idx()).

It's less obvious to the user, but seeing 2 events being rotated so they 
each have ~50% coverage should provide a hint that the user did 
something wrong, so I think that would work.

Thanks,

Neil
-- 
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm 
Technologies Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project.

  reply	other threads:[~2017-02-06 19:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-03 22:31 [PATCH v9] perf: add qcom l2 cache perf events driver Neil Leeder
2017-02-06 15:48 ` Mark Rutland
2017-02-06 19:11   ` Leeder, Neil [this message]
2017-02-07 10:43     ` Mark Rutland

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=8d61d7fb-71d0-190e-26e3-a72e98cfb10d@codeaurora.org \
    --to=nleeder@codeaurora.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /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).