Re: [PATCH 1/2] x86/fpu: Rename "dynamic" XSTATEs to "independent"

From: Peter Zijlstra
Date: Fri Jun 11 2021 - 04:01:57 EST


On Thu, Jun 10, 2021 at 10:13:36PM -0700, Andy Lutomirski wrote:
> The salient feature of "dynamic" XSTATEs is that they are not part of the
> main task XSTATE buffer. The fact that they are dynamically allocated is
> irrelevant and will become quite confusing when user math XSTATEs start
> being dynamically allocated. Rename them to "independent" because they
> are independent of the main XSTATE code.
>
> This is just a search-and-replace with some whitespace updates to keep
> things aligned.
>
> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
> ---
> arch/x86/events/intel/lbr.c | 6 +--
> arch/x86/include/asm/fpu/xstate.h | 14 +++----
> arch/x86/kernel/fpu/xstate.c | 62 +++++++++++++++----------------
> 3 files changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
> index 76dbab6ac9fb..0189807fc3c1 100644
> --- a/arch/x86/events/intel/lbr.c
> +++ b/arch/x86/events/intel/lbr.c
> @@ -491,7 +491,7 @@ static void intel_pmu_arch_lbr_xrstors(void *ctx)
> {
> struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
>
> - copy_kernel_to_dynamic_supervisor(&task_ctx->xsave, XFEATURE_MASK_LBR);
> + copy_kernel_to_independent_supervisor(&task_ctx->xsave, XFEATURE_MASK_LBR);
> }
>
> static __always_inline bool lbr_is_reset_in_cstate(void *ctx)
> @@ -576,7 +576,7 @@ static void intel_pmu_arch_lbr_xsaves(void *ctx)
> {
> struct x86_perf_task_context_arch_lbr_xsave *task_ctx = ctx;
>
> - copy_dynamic_supervisor_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
> + copy_independent_supervisor_to_kernel(&task_ctx->xsave, XFEATURE_MASK_LBR);
> }
>
> static void __intel_pmu_lbr_save(void *ctx)
> @@ -978,7 +978,7 @@ static void intel_pmu_arch_lbr_read_xsave(struct cpu_hw_events *cpuc)
> intel_pmu_store_lbr(cpuc, NULL);
> return;
> }
> - copy_dynamic_supervisor_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
> + copy_independent_supervisor_to_kernel(&xsave->xsave, XFEATURE_MASK_LBR);
>
> intel_pmu_store_lbr(cpuc, xsave->lbr.entries);
> }

Yesterday tglx proposed the *save*_to_{user,kernel}() and
*rstor*_from_{user,kernel}() namespace for pretty much every other such
function.

And while I agree that independent_supervisor beats dynamic_supervisor
for a name, they're both stupid long :-(

I don't suppose we can simply use xsaves_to_kernel()
xstrors_from_kernel() and add some magic to their respective mask
handling to ensure that a mask belongs to only 1 (of 3) types.

int types = 0;

if (mask & xfeatures_mask_user())
type++;
if (mask & xfeatures_mask_supervisor())
types++;
if (mask & xfeatures_mask_independent())
types++;

if (WARN_ON_ONCE(type != 1))
return;

?