[PATCH v3 2/2] perf,x86: skip intel_cqm_stable if CMT is not present in a CPU model

[PATCH v3 2/2] perf,x86: skip intel_cqm_stable if CMT is not present in a CPU model

[Kanaka Juvva]

CMT and MBM are complementary technologies. One technology doesn't
imply the other technology. If CMT is not present in your CPU model
intel_cqm_stable() won't be called when computing a free RMID. This
is because, LLC_OCCUPANCY reading in this case doesn't apply and
shouldn't be used a criteria for freeing or picking an RMID.

Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
---
 arch/x86/kernel/cpu/perf_event_intel_cqm.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
index 63eb68b..7aa3bc0 100644
--- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c
+++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
@@ -125,6 +125,7 @@ struct cqm_rmid_entry {
 	enum rmid_recycle_state state;
 	struct list_head list;
 	unsigned long queue_time;
+	bool config;
 };
 
 /*
@@ -232,6 +233,7 @@ static int intel_cqm_setup_rmid_cache(void)
 
 		INIT_LIST_HEAD(&entry->list);
 		entry->rmid = r;
+		entry->config = false;
 		cqm_rmid_ptrs[r] = entry;
 
 		list_add_tail(&entry->list, &cqm_rmid_free_lru);
@@ -568,7 +570,8 @@ static bool intel_cqm_rmid_stabilize(unsigned int *available)
 	/*
 	 * Test whether an RMID is free for each package.
 	 */
-	on_each_cpu_mask(&cqm_cpumask, intel_cqm_stable, NULL, true);
+	if (entry->config)
+		on_each_cpu_mask(&cqm_cpumask, intel_cqm_stable, NULL, true);
 
 	list_for_each_entry_safe(entry, tmp, &cqm_rmid_limbo_lru, list) {
 		/*
@@ -1003,6 +1006,12 @@ static void intel_cqm_event_start(struct perf_event *event, int mode)
 	}
 
 	state->rmid = rmid;
+	if (event->attr.config & QOS_L3_OCCUP_EVENT_ID) {
+		struct cqm_rmid_entry *entry;
+
+		entry = __rmid_entry(rmid);
+		entry->config = true;
+	}
 	wrmsr(MSR_IA32_PQR_ASSOC, rmid, state->closid);
 }
 
-- 
2.1.0

Re: [PATCH v3 2/2] perf,x86: skip intel_cqm_stable if CMT is not present in a CPU model

[Matt Fleming]

On Sat, 08 Aug, at 12:36:19AM, Kanaka Juvva wrote:
> CMT and MBM are complementary technologies. One technology doesn't
> imply the other technology. If CMT is not present in your CPU model
> intel_cqm_stable() won't be called when computing a free RMID. This
> is because, LLC_OCCUPANCY reading in this case doesn't apply and
> shouldn't be used a criteria for freeing or picking an RMID.
> 
> Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_cqm.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

Shouldn't this patch be the first one in the series?

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v3 2/2] perf,x86: skip intel_cqm_stable if CMT is not present in a CPU model

[Matt Fleming]

On Sat, 08 Aug, at 12:36:19AM, Kanaka Juvva wrote:
> CMT and MBM are complementary technologies. One technology doesn't
> imply the other technology. If CMT is not present in your CPU model
> intel_cqm_stable() won't be called when computing a free RMID. This
> is because, LLC_OCCUPANCY reading in this case doesn't apply and
> shouldn't be used a criteria for freeing or picking an RMID.

This could do with some additional information.

The main point is that when you re-use an RMID for reading MBM values
that RMID doesn't "remember" the last MBM value it read, they're
instantenous and "non-sticky", so you can skip the recycling. That's
not the case for reading CMT values since old lines in the cache are
still tagged with the RMID you want to re-use and they will be
included the CMT value for that RMID until they're naturally evicted
from the cache.

> Signed-off-by: Kanaka Juvva <kanaka.d.juvva@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/perf_event_intel_cqm.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/cpu/perf_event_intel_cqm.c b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> index 63eb68b..7aa3bc0 100644
> --- a/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> +++ b/arch/x86/kernel/cpu/perf_event_intel_cqm.c
> @@ -125,6 +125,7 @@ struct cqm_rmid_entry {
>  	enum rmid_recycle_state state;
>  	struct list_head list;
>  	unsigned long queue_time;
> +	bool config;
>  };
  
This isn't the best field name since 'config' doesn't explain that "We
don't need to stabilize this RMID because it's only been used for
reading MBM values". What about 'needs_stabilizing'?

>  /*
> @@ -232,6 +233,7 @@ static int intel_cqm_setup_rmid_cache(void)
>  
>  		INIT_LIST_HEAD(&entry->list);
>  		entry->rmid = r;
> +		entry->config = false;
>  		cqm_rmid_ptrs[r] = entry;
>  
>  		list_add_tail(&entry->list, &cqm_rmid_free_lru);
> @@ -568,7 +570,8 @@ static bool intel_cqm_rmid_stabilize(unsigned int *available)
>  	/*
>  	 * Test whether an RMID is free for each package.
>  	 */
> -	on_each_cpu_mask(&cqm_cpumask, intel_cqm_stable, NULL, true);
> +	if (entry->config)
> +		on_each_cpu_mask(&cqm_cpumask, intel_cqm_stable, NULL, true);
>  

'entry' is a loop variable and so you're just checking the ->config
value of the last RMID on the limbo list. If you've got a mixture of
MBM and CMT RMIDs this might do the wrong thing.

You can only skip the stabilization if there are no CMT RMIDs on the
limbo lru at all.

Also, you need more changes to this function because you're still doing the
minimum queue time delay, even for MBM events, e.g.

	*available = 0;
	list_for_each_entry(entry, &cqm_rmid_limbo_lru, list) {
		unsigned long min_queue_time;
		unsigned long now = jiffies;

		/*
		 * We hold RMIDs placed into limbo for a minimum queue
		 * time. Before the minimum queue time has elapsed we do
		 * not recycle RMIDs.
		 *
		 * The reasoning is that until a sufficient time has
		 * passed since we stopped using an RMID, any RMID
		 * placed onto the limbo list will likely still have
		 * data tagged in the cache, which means we'll probably
		 * fail to recycle it anyway.
		 *
		 * We can save ourselves an expensive IPI by skipping
		 * any RMIDs that have not been queued for the minimum
		 * time.
		 */
		min_queue_time = entry->queue_time +
			msecs_to_jiffies(__rmid_queue_time_ms);

		if (time_after(min_queue_time, now))
			break;

		entry->state = RMID_AVAILABLE;
		(*available)++;
	}


You can mark the RMID as availble irrespective of the time it has been
sat on the limbo list if that RMID has only been used for MBM events.

>  		/*
> @@ -1003,6 +1006,12 @@ static void intel_cqm_event_start(struct perf_event *event, int mode)
>  	}
>  
>  	state->rmid = rmid;
> +	if (event->attr.config & QOS_L3_OCCUP_EVENT_ID) {
> +		struct cqm_rmid_entry *entry;
> +
> +		entry = __rmid_entry(rmid);
> +		entry->config = true;
> +	}
>  	wrmsr(MSR_IA32_PQR_ASSOC, rmid, state->closid);
>  }
  
Because RMIDs can be rotated between events, we need to maintain this
metadata in other locations, not jut intel_cqm_event_start().

Take a look at intel_cqm_xchg_rmid() and intel_cqm_setup_event().
Anywhere we assign an RMID to an event we potentially need to update
whether or not the RMID will need recycling.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH v3 2/2] perf,x86: skip intel_cqm_stable if CMT is not present in a CPU model

[Matt Fleming]

On Tue, 18 Aug, at 06:09:01PM, Matt Fleming wrote:
> 
> You can only skip the stabilization if there are no CMT RMIDs on the
> limbo lru at all.

I need to provide a little more detail here...

There are actually 3 cases you need to consider when you make the MBM
changes to intel_cqm_rmid_stabilize(),

  1. RMIDs on the limbo lru are *only* CMT
  2. RMIDs on the limbo lru are *only* MBM
  3. RMIDs on the limbo lru are a mixture of CMT *and* MBM

Case 1 is exactly what is supported today and so that functionality
needs to remain unchanged. For case 2 you can completely skip the SMP IPI
stabilization and move all RMIDs from the limbo lru to the free lru,
which is similar in spirit to your patch.

Case 3 is by far the most interesting. The simplest way to handle it
is to skip any MBM RMIDs on the limbo lru inside of the SMP IPI
callback, intel_cqm_stable().

But that's pretty suboptimal if you have lots of MBM RMIDs and few CMT
RMIDs on the limbo lru because you'd be iterating most of the entries
in the callback function for no reason - you'd just skip them.

Instead what I suggest you explore is removing each MBM RMID from the
limbo lru and putting it on a function-local list on the stack in
intel_cqm_rmid_stabilize(). Since you are guaranteed to be allowed to
put the MBM RMID on the free lru (we don't need to stabilize it, so
the stabilzation can't fail) there's no need to worry about error
cases or anything like that.

-- 
Matt Fleming, Intel Open Source Technology Center