Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1679442

Re: [PATCH 08/21] x86/intel_rdt/cqm: Add RMID(Resource monitoring ID) management

Show all headers | View raw

On Mon, 26 Jun 2017, Vikas Shivappa wrote:
> +static u64 __rmid_read(u32 rmid, u32 eventid)
> +{
> +	u64 val;
> +
> +	wrmsr(MSR_IA32_QM_EVTSEL, eventid, rmid);
> +	rdmsrl(MSR_IA32_QM_CTR, val);

The calling convention of this function needs to be documented. It's
obvious that it needs to be serialized ....

> +
> +	/*
> +	 * Aside from the ERROR and UNAVAIL bits, the return value is the
> +	 * count for this @eventid tagged with @rmid.
> +	 */
> +	return val;
> +}
> +
> +/*
> + * Test whether an RMID is dirty(occupancy > threshold_occupancy)
> + */
> +static void intel_cqm_stable(void *arg)
> +{
> +	struct rmid_entry *entry;
> +	u64 val;
> +
> +	/*
> +	 * Since we are in the IPI already lets mark all the RMIDs
> +	 * that are dirty

This comment is crap. It suggests: Let's do it while we are here anyway.

But that's not true. The IPI is issued solely to figure out which RMIDs are
dirty.

> +	 */
> +	list_for_each_entry(entry, &rmid_limbo_lru, list) {

Since this is executed on multiple CPUs, that needs an explanation why that
list is safe to iterate w/o explicit protection here.

> +		val = __rmid_read(entry->rmid, QOS_L3_OCCUP_EVENT_ID);
> +		if (val > intel_cqm_threshold)
> +			entry->state = RMID_DIRTY;
> +	}
> +}
> +
> +/*
> + * Scan the limbo list and move all entries that are below the
> + * intel_cqm_threshold to the free list.
> + * Return "true" if the limbo list is empty, "false" if there are
> + * still some RMIDs there.
> + */
> +static bool try_freeing_limbo_rmid(void)
> +{
> +	struct rmid_entry *entry, *tmp;
> +	struct rdt_resource *r;
> +	cpumask_var_t cpu_mask;
> +	struct rdt_domain *d;
> +	bool ret = true;
> +
> +	if (list_empty(&rmid_limbo_lru))
> +		return ret;
> +
> +	if (!zalloc_cpumask_var(&cpu_mask, GFP_KERNEL))
> +		return false;
> +
> +	r = &rdt_resources_all[RDT_RESOURCE_L3];
> +
> +	list_for_each_entry(d, &r->domains, list)
> +		cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask);
> +
> +	/*
> +	 * Test whether an RMID is free for each package.

That wants a bit of explanation at some place why RMIDs have global
scope. That's a pure implementation decision because from a hardware POV
RMIDs have package scope. We could use the same RMID on different packages
for different purposes.

> +	 */
> +	on_each_cpu_mask(cpu_mask, intel_cqm_stable, NULL, true);
> +
> +	list_for_each_entry_safe(entry, tmp, &rmid_limbo_lru, list) {
> +		/*
> +		 * Ignore the RMIDs that are marked dirty and reset the
> +		 * state to check for being dirty again later.

Ignore? -EMAKESNOSENSE

> +		 */
> +		if (entry->state == RMID_DIRTY) {
> +			entry->state = RMID_CHECK;
> +			ret = false;
> +			continue;
> +		}
> +		list_del(&entry->list);
> +		list_add_tail(&entry->list, &rmid_free_lru);
> +	}
> +
> +	free_cpumask_var(cpu_mask);

...

> +void free_rmid(u32 rmid)
> +{
> +	struct rmid_entry *entry;
> +
> +	lockdep_assert_held(&rdtgroup_mutex);
> +
> +	WARN_ON(!rmid);
> +	entry = __rmid_entry(rmid);
> +
> +	entry->state = RMID_CHECK;
> +
> +	if (rdt_mon_features & (1 << QOS_L3_OCCUP_EVENT_ID))
> +		list_add_tail(&entry->list, &rmid_limbo_lru);
> +	else
> +		list_add_tail(&entry->list, &rmid_free_lru);

Thinking a bit more about that limbo mechanics.

In case that a RMID was never used on a particular package, the state check
forces an IPI on all packages unconditionally. That's suboptimal at least.

We know on which package a given RMID was used, so we could restrict the
checks to exactly these packages, but I'm not sure it's worth the
trouble. We might at least document that and explain why this is
implemented in that way.

Thanks,

	tglx

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH V1 00/21] x86/cqm3: Resctrl based cqm Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
  [PATCH 16/21] x86/intel_rdt/cqm: Add mount,umount support Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 16/21] x86/intel_rdt/cqm: Add mount,umount support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 15:30 +0200
  [PATCH 20/21] x86/intel_rdt/mbm: Add mbm counter initialization Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
  [PATCH 13/21] x86/intel_rdt/cqm: Add cpus file support Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 13/21] x86/intel_rdt/cqm: Add cpus file support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 13:20 +0200
    Re: [PATCH 13/21] x86/intel_rdt/cqm: Add cpus file support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 14:40 +0200
  [PATCH 15/21] x86/intel_rdt/cqm: Add rmdir support Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 15/21] x86/intel_rdt/cqm: Add rmdir support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 15:20 +0200
  [PATCH 03/21] x86/intel_rdt/cqm: Documentation for resctrl based RDT Monitoring Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
  [PATCH 21/21] x86/intel_rdt/mbm: Handle counter overflow Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 21/21] x86/intel_rdt/mbm: Handle counter overflow Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 16:00 +0200
  [PATCH 14/21] x86/intel_rdt/cqm: Add mon_data Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 14/21] x86/intel_rdt/cqm: Add mon_data Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 14:50 +0200
  [PATCH 17/21] x86/intel_rdt/cqm: Add sched_in support Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 17/21] x86/intel_rdt/cqm: Add sched_in support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 15:40 +0200
  [PATCH 12/21] x86/intel_rdt/cqm: Add tasks file support Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 12/21] x86/intel_rdt/cqm: Add tasks file support Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 13:10 +0200
  [PATCH 08/21] x86/intel_rdt/cqm: Add RMID(Resource monitoring ID) management Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-06-26 21:00 +0200
    Re: [PATCH 08/21] x86/intel_rdt/cqm: Add RMID(Resource monitoring  ID) management Thomas Gleixner <tglx@linutronix.de> - 2017-07-02 12:10 +0200
      Re: [PATCH 08/21] x86/intel_rdt/cqm: Add RMID(Resource monitoring  ID) management Thomas Gleixner <tglx@linutronix.de> - 2017-07-03 12:00 +0200

csiph-web