linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Noah Goldstein <goldstein.w.n@gmail.com>
To: unlisted-recipients:; (no To-header on input)
Cc: goldstein.w.n@gmail.com, tglx@linutronix.de, mingo@redhat.com,
	bp@alien8.de, x86@kernel.org, hpa@zytor.com, luto@kernel.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2 2/2] x86/xstate: Make AVX512 status tracking more accurate
Date: Fri, 15 Oct 2021 15:47:17 -0500	[thread overview]
Message-ID: <20211015204713.2855996-2-goldstein.w.n@gmail.com> (raw)
In-Reply-To: <20210920053951.4093668-1-goldstein.w.n@gmail.com>

This patch splits the timestamps for tracking the AVX512 status into
'avx512_ZMM_Hi256_timestamp' and 'avx512_Hi16_ZMM_timestamp'. They are
used for tracking XFEATURE_ZMM_Hi256 and XFEATURE_Hi16_ZMM use
respectively.

The purpose of tracking the AVX512 status is to convey information
about possible frequency throttling. The current implementation has
false positives on XFEATURE_OPMASK use and any usage of the hi16 xmm
and ymm registers which are included in the XFEATURE_Hi16_ZMM set as
neither will cause frequency throttling.

This patches implementation avoids to add more clarity to the
output. The 'avx512_ZMM_Hi256_timestamp' will not have false positives
so its value will at least be indicative frequency throttling. Since
'avx512_Hi16_ZMM_timestamp' can still indicate frequency throttling
from zmm16...zmm31 use though had false positives it is separated.

Signed-off-by: Noah Goldstein <goldstein.w.n@gmail.com>
---
Because (Hi16_ZMM | ZMM_Hi256) will likely be full of false positives
on any machine that support avx512 I split the avx512_timestamp.

One to track Hi16_ZMM and one for ZMM_Hi256. My though is it's not
good to add more holes where the status doesn't report, but also have
fields that will be useful and not be burdened by false positives.

This might be overdoing it. If so we can either use a both of both or
just ZMM_Hi256.

 arch/x86/include/asm/fpu/types.h | 16 +++++++++---
 arch/x86/kernel/fpu/xstate.c     | 45 ++++++++++++++++++++++++++------
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
index f5a38a5f3ae1..cb10909fa3da 100644
--- a/arch/x86/include/asm/fpu/types.h
+++ b/arch/x86/include/asm/fpu/types.h
@@ -330,11 +330,21 @@ struct fpu {
 	unsigned int			last_cpu;
 
 	/*
-	 * @avx512_timestamp:
+	 * @avx512_ZMM_Hi256_timestamp:
 	 *
-	 * Records the timestamp of AVX512 use during last context switch.
+	 * Records the timestamp of AVX512 use in the ZMM_Hi256 xfeature
+	 * set. This include zmm0...zmm15.
 	 */
-	unsigned long			avx512_timestamp;
+	unsigned long			avx512_ZMM_Hi256_timestamp;
+
+	/*
+	 * @avx512_Hi16_ZMM_timestamp:
+	 *
+	 * Records the timestamp of AVX512 use in the Hi16_ZMM xfeature
+	 * set. This includes usage of any of the hi16 xmm, ymm, or zmm
+	 * registers.
+	 */
+	unsigned long			avx512_Hi16_ZMM_timestamp;
 
 	/*
 	 * @state:
diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
index 00b495914be2..5b0ff609af2f 100644
--- a/arch/x86/kernel/fpu/xstate.c
+++ b/arch/x86/kernel/fpu/xstate.c
@@ -1246,7 +1246,7 @@ void xrstors(struct xregs_state *xstate, u64 mask)
 }
 
 /*
- * Track of the state of desired avx architecture features.
+ * Track of the state of desired avx related xfeatures.
  */
 void fpu_update_avx_timestamp(struct fpu *fpu)
 {
@@ -1254,18 +1254,28 @@ void fpu_update_avx_timestamp(struct fpu *fpu)
 	 * AVX512 state is tracked here because its use is known to slow
 	 * the max clock speed of the core.
 	 */
-	if (fpu->state.xsave.header.xfeatures & XFEATURE_MASK_AVX512)
-		fpu->avx512_timestamp = jiffies;
+
+	/*
+	 * Store a separate state for ZMM_Hi256 and Hi16_ZMM xfeature use.
+	 * If ZMM_Hi256 is used the machine has certainly used a zmm
+	 * register.  Hi16_ZMM, however, has false positives on usage of
+	 * hi16 xmm and ymm registers.
+	 */
+	if (fpu->state.xsave.header.xfeatures & XFEATURE_MASK_ZMM_Hi256)
+		fpu->avx512_ZMM_Hi256_timestamp = jiffies;
+	if (fpu->state.xsave.header.xfeatures & XFEATURE_MASK_Hi16_ZMM)
+		fpu->avx512_Hi16_ZMM_timestamp = jiffies;
 }
 
+
 #ifdef CONFIG_PROC_PID_ARCH_STATUS
+
 /*
- * Report the amount of time elapsed in millisecond since last AVX512
- * use in the task.
+ * Helper function for computing proper output for avx512_status
+ * timestamp.
  */
-static void avx512_status(struct seq_file *m, struct task_struct *task)
+static long avx_status_compute_delta(unsigned long timestamp)
 {
-	unsigned long timestamp = READ_ONCE(task->thread.fpu.avx512_timestamp);
 	long delta;
 
 	if (!timestamp) {
@@ -1282,8 +1292,27 @@ static void avx512_status(struct seq_file *m, struct task_struct *task)
 			delta = LONG_MAX;
 		delta = jiffies_to_msecs(delta);
 	}
+	return delta;
+}
 
-	seq_put_decimal_ll(m, "AVX512_elapsed_ms:\t", delta);
+/*
+ * Report the amount of time elapsed in millisecond since last AVX512
+ * use in the task.
+ */
+static void avx512_status(struct seq_file *m, struct task_struct *task)
+{
+	unsigned long timestamp;
+	long delta_ZMM_Hi256, delta_Hi16_ZMM;
+
+	timestamp = READ_ONCE(task->thread.fpu.avx512_ZMM_Hi256_timestamp);
+	delta_ZMM_Hi256 = avx_status_compute_delta(timestamp);
+
+	timestamp = READ_ONCE(task->thread.fpu.avx512_Hi16_ZMM_timestamp);
+	delta_Hi16_ZMM = avx_status_compute_delta(timestamp);
+
+	seq_put_decimal_ll(m, "AVX512_ZMM_Hi256_elapsed_ms:\t", delta_ZMM_Hi256);
+	seq_putc(m, '\n');
+	seq_put_decimal_ll(m, "AVX512_Hi16_ZMM_elapsed_ms:\t", delta_Hi16_ZMM);
 	seq_putc(m, '\n');
 }
 
-- 
2.25.1


  parent reply	other threads:[~2021-10-15 20:57 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-20  5:39 [PATCH v1] x86/fpu: Remove opmask state from avx512_timestamp check Noah Goldstein
2021-09-27 18:02 ` Noah Goldstein
2021-10-13 22:36   ` Noah Goldstein
2021-10-14  8:28     ` Borislav Petkov
2021-10-14 15:49       ` Noah Goldstein
2021-10-15 14:40         ` Borislav Petkov
2021-10-15 15:25         ` Dave Hansen
2021-10-15 17:30           ` Noah Goldstein
2021-10-15 20:47 ` [PATCH v2 1/2] x86/fpu: Add helper function for tracking AVX512 status Noah Goldstein
2021-10-26 23:15   ` Noah Goldstein
2021-10-15 20:47 ` Noah Goldstein [this message]
2021-10-27 11:07   ` [PATCH v2 2/2] x86/xstate: Make AVX512 status tracking more accurate Borislav Petkov
2021-10-27 16:28     ` Noah Goldstein
2021-10-27 16:58       ` Borislav Petkov
2021-10-27 17:18         ` Noah Goldstein
2021-10-27 16:26 ` [PATCH v3 1/2] x86/fpu: Add helper function for tracking AVX512 status Noah Goldstein
2021-10-27 16:26   ` [PATCH v3 2/2] x86/xstate: Make AVX512 status tracking more accurate Noah Goldstein
2021-10-27 17:11     ` Borislav Petkov
2021-10-27 17:37       ` Noah Goldstein
2021-10-27 17:45         ` Borislav Petkov
2021-10-27 18:21           ` Noah Goldstein
2021-11-03 20:22             ` Noah Goldstein
2021-11-03 20:48               ` Borislav Petkov
2021-11-03 20:56                 ` Noah Goldstein
2021-10-27 17:17 ` [PATCH v4 1/2] x86/fpu: Add helper function for tracking AVX512 status Noah Goldstein
2021-10-27 17:17   ` [PATCH v4 2/2] x86/xstate: Make AVX512 status tracking more accurate Noah Goldstein
2021-10-27 18:21 ` [PATCH v5 1/2] x86/fpu: Add helper function for tracking AVX512 status Noah Goldstein
2021-10-27 18:21   ` [PATCH v5 2/2] x86/xstate: Make AVX512 status tracking more accurate Noah Goldstein
2021-11-16 16:21     ` [PATCH] x86/fpu: Correct AVX512 state tracking Borislav Petkov
2021-11-16 16:49       ` Noah Goldstein
2021-11-17 18:29 ` [tip: x86/fpu] " tip-bot2 for Noah Goldstein

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=20211015204713.2855996-2-goldstein.w.n@gmail.com \
    --to=goldstein.w.n@gmail.com \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@redhat.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;
as well as URLs for NNTP newsgroup(s).