Re: [v2 PATCH 2/2] powerpc: Enable CPU_FTR_ASYM_SMT for interleaved big-cores

On Wed, Jul 04, 2018 at 01:45:05PM +0530, Gautham R Shenoy wrote:

Hi Murilo,

Thanks for the review.

On Tue, Jul 03, 2018 at 02:53:46PM -0300, Murilo Opsfelder Araujo wrote:
[..snip..]

quoted

quoted

-        /* Initialize CPU <=> thread mapping/
+	if (has_interleaved_big_core) {
+		int key = __builtin_ctzl(CPU_FTR_ASYM_SMT);
+
+		cur_cpu_spec->cpu_features |= CPU_FTR_ASYM_SMT;
+		static_branch_enable(&cpu_feature_keys[key]);
+		pr_info("Detected interleaved big-cores\n");
+	}

Shouldn't we use cpu_has_feature(CPU_FTR_ASYM_SMT) before setting

quoted

it?

Are you suggesting that we do the following?

    if (has_interleaved_big_core &&
        !cpu_has_feature(CPU_FTR_ASYM_SMT)) {
	...
    }
    
Currently CPU_FTR_ASYM_SMT is set at compile time for only POWER7
where running the tasks on lower numbered threads give us the benefit
of SMT thread folding. Interleaved big core is a feature introduced
only on POWER9. Thus, we know that CPU_FTR_ASYM_SMT is not set in
cpu_features at this point.

Since we're setting CPU_FTR_ASYM_SMT, it doesn't make sense to use
cpu_has_feature(CPU_FTR_ASYM_SMT).  I thought cpu_has_feature() held all
available features (not necessarily enabled) that we could check before
setting or enabling such feature.  I think I misread it.  Sorry.

quoted

quoted

+
+	/* Initialize CPU <=> thread mapping/
 	 *
 	 * WARNING: We assume that the number of threads is the same for
 	 * every CPU in the system. If that is not the case, then some code
-- 
1.9.4

-- 
Murilo

--
Thanks and Regards
gautham.

-- 
Murilo