Re: [PATCH 12/13] powerpc/64: runlatch CTRL[RUN] set optimisation

[Michael Ellerman <mpe@ellerman.id.au>]

Nicholas Piggin <npiggin@gmail.com> writes:
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index baae104b16c7..f587c1fdf981 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -1960,11 +1960,25 @@ void show_stack(struct task_struct *tsk, unsigned long *stack)
>  void notrace __ppc64_runlatch_on(void)
>  {
>  	struct thread_info *ti = current_thread_info();
> -	unsigned long ctrl;
>  
> -	ctrl = mfspr(SPRN_CTRLF);
> -	ctrl |= CTRL_RUNLATCH;
> -	mtspr(SPRN_CTRLT, ctrl);
> +	if (cpu_has_feature(CPU_FTR_ARCH_206)) {
> +		/*
> +		 * Least significant bit (RUN) is the only writable bit of
> +		 * the CTRL register, so we can avoid mfspr. 2.06 is not the
> +		 * earliest ISA where this is the case, but it's convenient.
> +		 */
> +		mtspr(SPRN_CTRLT, CTRL_RUNLATCH);
> +	} else {
> +		unsigned long ctrl;
> +
> +		/*
> +		 * Some architectures (e.g., Cell) have writable fields other
> +		 * than RUN, so do the read-modify-write.
> +		 */
> +		ctrl = mfspr(SPRN_CTRLF);
> +		ctrl |= CTRL_RUNLATCH;
> +		mtspr(SPRN_CTRLT, ctrl);
> +	}

Does the generated code look any good if you do something like:

        unsigned long ctrl;

	ctrl = 0;
	if (!cpu_has_feature(CPU_FTR_ARCH_206))
		ctrl = mfspr(SPRN_CTRLF);

	ctrl |= CTRL_RUNLATCH;
        mtspr(SPRN_CTRLT, ctrl);


That would offend me slightly less ;)

cheers