linux-kernel - Re: [PATCH v8 1/3] x86/fpu: track AVX-512 usage of tasks

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 1 Feb 2019 13:30:49 +0800
From:   "Li, Aubrey" <aubrey.li@...ux.intel.com>
To:     Aubrey Li <aubrey.li@...el.com>, tglx@...utronix.de,
        mingo@...hat.com, peterz@...radead.org, hpa@...or.com
Cc:     ak@...ux.intel.com, tim.c.chen@...ux.intel.com,
        dave.hansen@...el.com, arjan@...ux.intel.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v8 1/3] x86/fpu: track AVX-512 usage of tasks

Hi Thomas,

Just a soft reminder in case you didn't get a chance to look at this version.

To address your concern about jiffies_64 on 32bit kernel, I use jiffies here
instead. And to address jiffies wrap around issue, I use the trick from kernel
micro time_before/after, that is, as long as the time difference <= LONG_MAX
(num of jiffies), no wrap around issue. And if time difference > LONG_MAX, 
then capped to LONG_MAX (24 days if HZ=1000). This should be long enough to 
determine how to schedule the tasks.

If this is not acceptable, I'll try to limit this feature on 64bit kernel.
Looking forward to your response.

Thanks,
-Aubrey

On 2019/1/18 2:38, Aubrey Li wrote:
> User space tools which do automated task placement need information
> about AVX-512 usage of tasks, because AVX-512 usage could cause core
> turbo frequency drop and impact the running task on the sibling CPU.
> 
> The XSAVE hardware structure has bits that indicate when valid state
> is present in registers unique to AVX-512 use.  Use these bits to
> indicate when AVX-512 has been in use and add per-task AVX-512 state
> timestamp tracking to context switch.
> 
> Well-written AVX-512 applications are expected to clear the AVX-512
> state when not actively using AVX-512 registers, so the tracking
> mechanism is imprecise and can theoretically miss AVX-512 usage during
> context switch. But it has been measured to be precise enough to be
> useful under real-world workloads like tensorflow and linpack.
> 
> If higher precision is required, suggest user space tools to use the
> PMU-based mechanisms in combination.
> 
> Signed-off-by: Aubrey Li <aubrey.li@...ux.intel.com>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Andi Kleen <ak@...ux.intel.com>
> Cc: Tim Chen <tim.c.chen@...ux.intel.com>
> Cc: Dave Hansen <dave.hansen@...el.com>
> Cc: Arjan van de Ven <arjan@...ux.intel.com>
> ---
>  arch/x86/include/asm/fpu/internal.h | 7 +++++++
>  arch/x86/include/asm/fpu/types.h    | 7 +++++++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
> index 69dcdf195b61..c74f7b5f6886 100644
> --- a/arch/x86/include/asm/fpu/internal.h
> +++ b/arch/x86/include/asm/fpu/internal.h
> @@ -411,6 +411,13 @@ static inline int copy_fpregs_to_fpstate(struct fpu *fpu)
>  {
>  	if (likely(use_xsave())) {
>  		copy_xregs_to_kernel(&fpu->state.xsave);
> +
> +		/*
> +		 * 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;
>  		return 1;
>  	}
>  
> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
> index 202c53918ecf..2e32e178e064 100644
> --- a/arch/x86/include/asm/fpu/types.h
> +++ b/arch/x86/include/asm/fpu/types.h
> @@ -302,6 +302,13 @@ struct fpu {
>  	 */
>  	unsigned char			initialized;
>  
> +	/*
> +	 * @avx512_timestamp:
> +	 *
> +	 * Records the timestamp of AVX512 use during last context switch.
> +	 */
> +	unsigned long			avx512_timestamp;
> +
>  	/*
>  	 * @state:
>  	 *
>