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

Groups > linux.kernel > #1701401 > unrolled thread

Re: [PATCH 16/28] x86/intel_rdt: Prepare to add RDT monitor cpus file support

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 16/28] x86/intel_rdt: Prepare to add RDT monitor cpus  file support Thomas Gleixner <tglx@linutronix.de> - 2017-08-01 21:50 +0200
    Re: [PATCH 16/28] x86/intel_rdt: Prepare to add RDT monitor cpus  file support Shivappa Vikas <vikas.shivappa@intel.com> - 2017-08-01 23:40 +0200

#1701401 — Re: [PATCH 16/28] x86/intel_rdt: Prepare to add RDT monitor cpus file support

On Tue, 25 Jul 2017, Vikas Shivappa wrote:
>  /*
>   * The cached intel_pqr_state is strictly per CPU and can never be
>   * updated from a remote CPU. Functions which modify the state
> @@ -49,6 +47,8 @@
>   */
>  DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);
>  
> +DEFINE_PER_CPU_READ_MOSTLY(struct intel_pqr_state, rdt_cpu_default);

Cacheline wise this is suboptimal. You have to touch two cachelines on each
context switch (at least for read).

If you make that:

struct intel_pqr_state {
	u32		default_cosid;
	u32		default_rmid;
	u32		cur_cosid;
	u32		cur_rmid;
};

DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);

then it's all together and you spare one cache line.

Thanks,

	tglx

[toc] | [next] | [standalone]

#1701521

On Tue, 1 Aug 2017, Thomas Gleixner wrote:

> On Tue, 25 Jul 2017, Vikas Shivappa wrote:
>>  /*
>>   * The cached intel_pqr_state is strictly per CPU and can never be
>>   * updated from a remote CPU. Functions which modify the state
>> @@ -49,6 +47,8 @@
>>   */
>>  DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);
>>
>> +DEFINE_PER_CPU_READ_MOSTLY(struct intel_pqr_state, rdt_cpu_default);
>
> Cacheline wise this is suboptimal. You have to touch two cachelines on each
> context switch (at least for read).
>
> If you make that:
>
> struct intel_pqr_state {
> 	u32		default_cosid;
> 	u32		default_rmid;
> 	u32		cur_cosid;
> 	u32		cur_rmid;
> };
>
> DEFINE_PER_CPU(struct intel_pqr_state, pqr_state);
>
> then it's all together and you spare one cache line.

Will fix..

Thanks,
Vikas

>
> Thanks,
>
> 	tglx
>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web