mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Ravi Bangoria <ravi.bangoria@amd.com>
To: <peterz@infradead.org>, <mingo@redhat.com>, <namhyung@kernel.org>
Cc: <ravi.bangoria@amd.com>, <acme@kernel.org>, <eranian@google.com>,
	<mark.rutland@arm.com>, <alexander.shishkin@linux.intel.com>,
	<jolsa@kernel.org>, <irogers@google.com>,
	<adrian.hunter@intel.com>, <kan.liang@linux.intel.com>,
	<tglx@linutronix.de>, <bp@alien8.de>,
	<dave.hansen@linux.intel.com>, <x86@kernel.org>, <hpa@zytor.com>,
	<linux-perf-users@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <santosh.shukla@amd.com>,
	<ananth.narayan@amd.com>, <sandipan.das@amd.com>
Subject: [PATCH v2 02/10] perf/amd/ibs: Remove pointless sample period check
Date: Fri, 6 Dec 2024 05:17:05 +0000	[thread overview]
Message-ID: <20241206051713.991-3-ravi.bangoria@amd.com> (raw)
In-Reply-To: <20241206051713.991-1-ravi.bangoria@amd.com>

Valid perf event sample period value for IBS PMUs (Fetch and Op both)
is limited to multiple of 0x10. perf_ibs_init() has this check:

  if (!event->attr.sample_freq && hwc->sample_period & 0x0f)
          return -EINVAL;

But it's broken since hwc->sample_period will always be 0 when
event->attr.sample_freq is 0 (irrespective of event->attr.freq value.)

One option to fix this is to change the condition:

  - if (!event->attr.sample_freq && hwc->sample_period & 0x0f)
  + if (!event->attr.freq && hwc->sample_period & 0x0f)

However, that will break all userspace tools which have been using IBS
event with sample_period not multiple of 0x10.

Another option is to remove the condition altogether and mask lower
nibble _silently_, same as what current code is inadvertently doing.
I'm preferring this approach as it keeps the existing behavior.

Acked-by: Namhyung Kim <namhyung@kernel.org>
Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
---
 arch/x86/events/amd/ibs.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 26dd5e5874f9..484606a9bf6b 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -294,13 +294,8 @@ static int perf_ibs_init(struct perf_event *event)
 		if (config & perf_ibs->cnt_mask)
 			/* raw max_cnt may not be set */
 			return -EINVAL;
-		if (!event->attr.sample_freq && hwc->sample_period & 0x0f)
-			/*
-			 * lower 4 bits can not be set in ibs max cnt,
-			 * but allowing it in case we adjust the
-			 * sample period to set a frequency.
-			 */
-			return -EINVAL;
+
+		/* Silently mask off lower nibble. IBS hw mandates it. */
 		hwc->sample_period &= ~0x0FULL;
 		if (!hwc->sample_period)
 			hwc->sample_period = 0x10;
-- 
2.47.0


  parent reply	other threads:[~2024-12-06  5:17 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-06  5:17 [PATCH v2 00/10] perf/amd/ibs: Fix sample period computations Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 01/10] perf/amd/ibs: Remove IBS_{FETCH|OP}_CONFIG_MASK macros Ravi Bangoria
2024-12-06  5:17 ` Ravi Bangoria [this message]
2024-12-06  5:17 ` [PATCH v2 03/10] perf/amd/ibs: Fix ->config to sample period calculation for OP pmu Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 04/10] perf/amd/ibs: Fix perf_ibs_op.cnt_mask for CurCnt Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 05/10] perf/amd/ibs: Don't allow freq mode event creation through ->config interface Ravi Bangoria
2024-12-06  8:47   ` Ingo Molnar
2024-12-06  9:07     ` Ravi Bangoria
2024-12-06  9:22       ` Ingo Molnar
2024-12-06  9:38         ` Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 06/10] perf/amd/ibs: Add pmu specific minimum period Ravi Bangoria
2024-12-06  8:42   ` Ingo Molnar
2024-12-06  8:47     ` Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 07/10] perf/amd/ibs: Add ->check_period() callback Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 08/10] perf/core: Introduce pmu->adjust_period() callback Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 09/10] perf test: Introduce DEFINE_SUITE_EXCLUSIVE() Ravi Bangoria
2024-12-06  5:17 ` [PATCH v2 10/10] perf test amd ibs: Add sample period unit test Ravi Bangoria
2024-12-06  5:24   ` Ravi Bangoria
2024-12-06  8:43     ` Ingo Molnar

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=20241206051713.991-3-ravi.bangoria@amd.com \
    --to=ravi.bangoria@amd.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=ananth.narayan@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=eranian@google.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --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=sandipan.das@amd.com \
    --cc=santosh.shukla@amd.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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

all inboxes | Powered by JetHome®