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

Groups > linux.kernel > #1441779 > unrolled thread

[PATCH 08/32] Define CONFIG_INTEL_RDT

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

  [PATCH 08/32] Define CONFIG_INTEL_RDT "Fenghua Yu" <fenghua.yu@intel.com> - 2016-07-13 00:10 +0200
    Re: [PATCH 08/32] Define CONFIG_INTEL_RDT Thomas Gleixner <tglx@linutronix.de> - 2016-07-13 12:30 +0200
      RE: [PATCH 08/32] Define CONFIG_INTEL_RDT "Yu, Fenghua" <fenghua.yu@intel.com> - 2016-07-13 20:10 +0200
        RE: [PATCH 08/32] Define CONFIG_INTEL_RDT "Yu, Fenghua" <fenghua.yu@intel.com> - 2016-07-13 23:20 +0200
        RE: [PATCH 08/32] Define CONFIG_INTEL_RDT Thomas Gleixner <tglx@linutronix.de> - 2016-07-13 23:20 +0200

#1441779 — [PATCH 08/32] Define CONFIG_INTEL_RDT

From: Vikas Shivappa <vikas.shivappa@linux.intel.com>

CONFIG_INTEL_RDT is defined. The option provides support for resource
allocation which is a sub-feature of Intel Resource Director Technology
(RDT).

Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com>
Signed-off-by: Fenghua Yu <fenghua.yu@intel.com>
Reviewed-by: Tony Luck <tony.luck@intel.com>
---
 arch/x86/Kconfig                 | 12 ++++++++++++
 arch/x86/include/asm/intel_rdt.h | 10 +++++++++-
 arch/x86/kernel/cpu/intel_rdt.c  | 12 +++++++-----
 include/linux/sched.h            |  3 ---
 4 files changed, 28 insertions(+), 9 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index d9a94da..231d44c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -408,6 +408,18 @@ config GOLDFISH
        def_bool y
        depends on X86_GOLDFISH
 
+config INTEL_RDT
+	bool "Intel Resource Director Technology support"
+	depends on X86_64 && CPU_SUP_INTEL
+	help
+	  This option provides support for resource allocation which is a
+	  sub-feature of Intel Resource Director Technology(RDT).
+	  Current implementation supports L3 cache allocation.
+	  Using this feature a user can specify the amount of L3 cache space
+	  into which an application can fill.
+
+	  Say N if unsure.
+
 if X86_32
 config X86_EXTENDED_PLATFORM
 	bool "Support for extended (non-PC) x86 platforms"
diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h
index afb6da3..6e20314 100644
--- a/arch/x86/include/asm/intel_rdt.h
+++ b/arch/x86/include/asm/intel_rdt.h
@@ -24,8 +24,16 @@ struct clos_cbm_table {
  * on scheduler hot path:
  * - This will stay as no-op unless we are running on an Intel SKU
  * which supports L3 cache allocation.
+ * - When support is present and enabled, does not do any
+ * IA32_PQR_MSR writes until the user starts really using the feature
+ * ie creates a rdtgroup directory and assigns a cache_mask thats
+ * different from the root rdtgroup's cache_mask.
  * - Caches the per cpu CLOSid values and does the MSR write only
- * when a task with a different CLOSid is scheduled in.
+ * when a task with a different CLOSid is scheduled in. That
+ * means the task belongs to a different rdtgroup.
+ * - Closids are allocated so that different rdtgroup directories
+ * with same cache_mask gets the same CLOSid. This minimizes CLOSids
+ * used and reduces MSR write frequency.
  */
 static inline void intel_rdt_sched_in(void)
 {
diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index ecaf8e6..525390a 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -108,17 +108,19 @@ static inline bool cache_alloc_supported(struct cpuinfo_x86 *c)
 	return false;
 }
 
-
 void __intel_rdt_sched_in(void *dummy)
 {
 	struct intel_pqr_state *state = this_cpu_ptr(&pqr_state);
-	u32 closid = current->closid;
 
-	if (closid == state->closid)
+	/*
+	 * Currently closid is always 0. When  user interface is added,
+	 * closid will come from user interface.
+	 */
+	if (state->closid == 0)
 		return;
 
-	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, closid);
-	state->closid = closid;
+	wrmsr(MSR_IA32_PQR_ASSOC, state->rmid, 0);
+	state->closid = 0;
 }
 
 /*
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 6005bba..253538f 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1761,9 +1761,6 @@ struct task_struct {
 	/* cg_list protected by css_set_lock and tsk->alloc_lock */
 	struct list_head cg_list;
 #endif
-#ifdef CONFIG_INTEL_RDT
-	u32 closid;
-#endif
 #ifdef CONFIG_FUTEX
 	struct robust_list_head __user *robust_list;
 #ifdef CONFIG_COMPAT
-- 
2.5.0

[toc] | [next] | [standalone]

#1442314

On Tue, 12 Jul 2016, Fenghua Yu wrote:

Subject: Define CONFIG_INTEL_RDT

That does not qualify as a proper patch subject

> From: Vikas Shivappa <vikas.shivappa@linux.intel.com>
> 
> CONFIG_INTEL_RDT is defined. 

That tells us what?

> --- a/arch/x86/include/asm/intel_rdt.h
> +++ b/arch/x86/include/asm/intel_rdt.h
> @@ -24,8 +24,16 @@ struct clos_cbm_table {
>   * on scheduler hot path:
>   * - This will stay as no-op unless we are running on an Intel SKU
>   * which supports L3 cache allocation.
> + * - When support is present and enabled, does not do any
> + * IA32_PQR_MSR writes until the user starts really using the feature
> + * ie creates a rdtgroup directory and assigns a cache_mask thats
> + * different from the root rdtgroup's cache_mask.
>   * - Caches the per cpu CLOSid values and does the MSR write only
> - * when a task with a different CLOSid is scheduled in.
> + * when a task with a different CLOSid is scheduled in. That
> + * means the task belongs to a different rdtgroup.
> + * - Closids are allocated so that different rdtgroup directories
> + * with same cache_mask gets the same CLOSid. This minimizes CLOSids
> + * used and reduces MSR write frequency.

How is this and the following changes related to $subject ?

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1442741

> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Wednesday, July 13, 2016 3:26 AM
> On Tue, 12 Jul 2016, Fenghua Yu wrote:
> 
> Subject: Define CONFIG_INTEL_RDT
> 
> That does not qualify as a proper patch subject
> 
> > From: Vikas Shivappa <vikas.shivappa@linux.intel.com>
> >
> > CONFIG_INTEL_RDT is defined.
> 
> That tells us what?
> 
> > --- a/arch/x86/include/asm/intel_rdt.h
> > +++ b/arch/x86/include/asm/intel_rdt.h
> > @@ -24,8 +24,16 @@ struct clos_cbm_table {
> >   * on scheduler hot path:
> >   * - This will stay as no-op unless we are running on an Intel SKU
> >   * which supports L3 cache allocation.
> > + * - When support is present and enabled, does not do any
> > + * IA32_PQR_MSR writes until the user starts really using the feature
> > + * ie creates a rdtgroup directory and assigns a cache_mask thats
> > + * different from the root rdtgroup's cache_mask.
> >   * - Caches the per cpu CLOSid values and does the MSR write only
> > - * when a task with a different CLOSid is scheduled in.
> > + * when a task with a different CLOSid is scheduled in. That
> > + * means the task belongs to a different rdtgroup.
> > + * - Closids are allocated so that different rdtgroup directories
> > + * with same cache_mask gets the same CLOSid. This minimizes CLOSids
> > + * used and reduces MSR write frequency.
> 
> How is this and the following changes related to $subject ?

No, this piece of code is not related to $subject.

Here is why this patch behaves like this:

This patch and actually first 12 patches are directly from last year's cgroup base CAT patch
series. The last year's patch series had gone 16 versions already. Because
the first 12 patches have been reviewed many times, we keep them untouched
(except removing cgroup code in patch 8 and some unused cdp code in patch 11)
and release other patches on top of the first 12 patches.

I fully agree this patch should be split if we want to have a good overall
patch series.

Thanks.

-Fenghua

[toc] | [prev] | [next] | [standalone]

#1442870

> From: Thomas Gleixner [mailto:tglx@linutronix.de]
> Sent: Wednesday, July 13, 2016 2:10 PM
> On Wed, 13 Jul 2016, Yu, Fenghua wrote:
> > Here is why this patch behaves like this:
> >
> > This patch and actually first 12 patches are directly from last year's
> > cgroup base CAT patch series. The last year's patch series had gone 16
> > versions already. Because the first 12 patches have been reviewed many
> > times, we keep them untouched (except removing cgroup code in patch 8
> > and some unused cdp code in patch 11) and release other patches on top
> > of the first 12 patches.
> 
> Which is not making the review any simpler. In order to understand the
> modifications I have to go back and page in the original stuff from last year
> once again. So I have to read the original patch first to understand the
> modifications and then get the overall picture of the new stuff. Please fold
> stuff back to the proper places so I can start reviewing this thing under the
> new design idea instead of twisting my brain around two designs.

Ok. I will do that.

> 
> > I fully agree this patch should be split if we want to have a good
> > overall patch series.
> 
> Good.
> 
> Thanks,
> 
> 	tglx

[toc] | [prev] | [next] | [standalone]

#1442871

On Wed, 13 Jul 2016, Yu, Fenghua wrote:
> Here is why this patch behaves like this:
> 
> This patch and actually first 12 patches are directly from last year's
> cgroup base CAT patch series. The last year's patch series had gone 16
> versions already. Because the first 12 patches have been reviewed many
> times, we keep them untouched (except removing cgroup code in patch 8 and
> some unused cdp code in patch 11) and release other patches on top of the
> first 12 patches.

Which is not making the review any simpler. In order to understand the
modifications I have to go back and page in the original stuff from last year
once again. So I have to read the original patch first to understand the
modifications and then get the overall picture of the new stuff. Please fold
stuff back to the proper places so I can start reviewing this thing under the
new design idea instead of twisting my brain around two designs.
 
> I fully agree this patch should be split if we want to have a good overall
> patch series.

Good.

Thanks,

	tglx

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web