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

Groups > linux.kernel > #1712562 > unrolled thread

[PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu

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 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu Vikas Shivappa <vikas.shivappa@linux.intel.com> - 2017-08-16 03:00 +0200
    Re: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during  hot cpu Thomas Gleixner <tglx@linutronix.de> - 2017-08-16 11:30 +0200
      RE: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during  hot cpu "Luck, Tony" <tony.luck@intel.com> - 2017-08-16 17:00 +0200
        RE: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during  hot cpu Thomas Gleixner <tglx@linutronix.de> - 2017-08-16 17:00 +0200
    [tip:x86/cache] x86/intel_rdt/mbm: Fix MBM overflow handler during  CPU hotplug tip-bot for Vikas Shivappa <tipbot@zytor.com> - 2017-08-16 12:20 +0200

#1712562 — [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu

When a CPU is dying, we cancel the worker and schedule a new worker on a
different CPU on the same domain. But if the timer is already about to
expire (say 0.99s) then we essentially double the interval.

We modify the hot cpu handling to cancel the delayed work on the dying
cpu and run the worker immediately on a different cpu in same domain. We
donot flush the worker because the MBM overflow worker reschedules the
worker on same CPU and scans the domain->cpu_mask to get the domain
pointer.

Reported-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com>
---
 arch/x86/kernel/cpu/intel_rdt.c          | 4 ++--
 arch/x86/kernel/cpu/intel_rdt.h          | 2 +-
 arch/x86/kernel/cpu/intel_rdt_monitor.c  | 4 ++--
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 97c8d83..b8dc141 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -447,7 +447,7 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d)
 
 	if (is_mbm_enabled()) {
 		INIT_DELAYED_WORK(&d->mbm_over, mbm_handle_overflow);
-		mbm_setup_overflow_handler(d);
+		mbm_setup_overflow_handler(d, MBM_OVERFLOW_INTERVAL);
 	}
 
 	return 0;
@@ -540,7 +540,7 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
 	} else if (r == &rdt_resources_all[RDT_RESOURCE_L3] &&
 		   cpu == d->mbm_work_cpu && is_mbm_enabled()) {
 		cancel_delayed_work(&d->mbm_over);
-		mbm_setup_overflow_handler(d);
+		mbm_setup_overflow_handler(d, 0);
 	}
 }
 
diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 4040bf1..3e48693 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -422,7 +422,7 @@ void mkdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
 				    struct rdt_domain *d);
 void mon_event_read(struct rmid_read *rr, struct rdt_domain *d,
 		    struct rdtgroup *rdtgrp, int evtid, int first);
-void mbm_setup_overflow_handler(struct rdt_domain *dom);
+void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms);
 void mbm_handle_overflow(struct work_struct *work);
 
 #endif /* _ASM_X86_INTEL_RDT_H */
diff --git a/arch/x86/kernel/cpu/intel_rdt_monitor.c b/arch/x86/kernel/cpu/intel_rdt_monitor.c
index d6bfdfd..8378785 100644
--- a/arch/x86/kernel/cpu/intel_rdt_monitor.c
+++ b/arch/x86/kernel/cpu/intel_rdt_monitor.c
@@ -417,9 +417,9 @@ void mbm_handle_overflow(struct work_struct *work)
 	mutex_unlock(&rdtgroup_mutex);
 }
 
-void mbm_setup_overflow_handler(struct rdt_domain *dom)
+void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms)
 {
-	unsigned long delay = msecs_to_jiffies(MBM_OVERFLOW_INTERVAL);
+	unsigned long delay = msecs_to_jiffies(delay_ms);
 	int cpu;
 
 	if (!static_branch_likely(&rdt_enable_key))
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 86a6979..b529f93 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -1140,7 +1140,7 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 	if (is_mbm_enabled()) {
 		r = &rdt_resources_all[RDT_RESOURCE_L3];
 		list_for_each_entry(dom, &r->domains, list)
-			mbm_setup_overflow_handler(dom);
+			mbm_setup_overflow_handler(dom, MBM_OVERFLOW_INTERVAL);
 	}
 
 	goto out;
-- 
1.9.1

[toc] | [next] | [standalone]

#1712794 — Re: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu

On Tue, 15 Aug 2017, Vikas Shivappa wrote:
> When a CPU is dying, we cancel the worker and schedule a new worker on a
> different CPU on the same domain. But if the timer is already about to
> expire (say 0.99s) then we essentially double the interval.
> 
> We modify the hot cpu handling to cancel the delayed work on the dying
> cpu and run the worker immediately on a different cpu in same domain. We
> donot flush the worker because the MBM overflow worker reschedules the
> worker on same CPU and scans the domain->cpu_mask to get the domain
> pointer.

You could alternatively use flush and make the worker code schedule the
work on a still online CPU in the domain instead of blindly rescheduling it
on the same CPU.

Thanks,

	tglx

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

#1713028 — RE: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu

> You could alternatively use flush and make the worker code schedule the
> work on a still online CPU in the domain instead of blindly rescheduling it
> on the same CPU.

We looked at that when you suggested flush. The problem is that we have
already deleted the current cpu from the bitmask for the domain. So the
worker code doesn't know which domain it is running on, so can't pick
another.

If we try to do the flush before dropping the cpu from the bitmask, then
the worker code doesn't have any reason to pick a different CPU.

Is there is some cheap "I'm running on a CPU that is in the process of going
offline" test that we could make in the worker code?

-Tony

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

#1713033 — RE: [PATCH 1/2] x86/intel_rdt/mbm: Fix MBM overflow handler during hot cpu

On Wed, 16 Aug 2017, Luck, Tony wrote:
> > You could alternatively use flush and make the worker code schedule the
> > work on a still online CPU in the domain instead of blindly rescheduling it
> > on the same CPU.
> 
> We looked at that when you suggested flush. The problem is that we have
> already deleted the current cpu from the bitmask for the domain. So the
> worker code doesn't know which domain it is running on, so can't pick
> another.
> 
> If we try to do the flush before dropping the cpu from the bitmask, then
> the worker code doesn't have any reason to pick a different CPU.
> 
> Is there is some cheap "I'm running on a CPU that is in the process of going
> offline" test that we could make in the worker code?

Don't think so. It was just a thought, but the code as provided is fine and
merged as is.

Thanks,

	tglx

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

#1712823 — [tip:x86/cache] x86/intel_rdt/mbm: Fix MBM overflow handler during CPU hotplug

Commit-ID:  bbc4615e0b7df5e21d0991adb4b2798508354924
Gitweb:     http://git.kernel.org/tip/bbc4615e0b7df5e21d0991adb4b2798508354924
Author:     Vikas Shivappa <vikas.shivappa@linux.intel.com>
AuthorDate: Tue, 15 Aug 2017 18:00:42 -0700
Committer:  Thomas Gleixner <tglx@linutronix.de>
CommitDate: Wed, 16 Aug 2017 12:05:41 +0200

x86/intel_rdt/mbm: Fix MBM overflow handler during CPU hotplug

When a CPU is dying, the overflow worker is canceled and rescheduled on a
different CPU in the same domain. But if the timer is already about to
expire this essentially doubles the interval which might result in a non
detected overflow.

Cancel the overflow worker and reschedule it immediately on a different CPU
in same domain. The work could be flushed as well, but that would
reschedule it on the same CPU.

[ tglx: Rewrote changelog once again ]

Reported-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: ravi.v.shankar@intel.com
Cc: tony.luck@intel.com
Cc: fenghua.yu@intel.com
Cc: peterz@infradead.org
Cc: eranian@google.com
Cc: vikas.shivappa@intel.com
Cc: ak@linux.intel.com
Cc: davidcc@google.com
Link: http://lkml.kernel.org/r/1502845243-20454-2-git-send-email-vikas.shivappa@linux.intel.com

---
 arch/x86/kernel/cpu/intel_rdt.c          | 4 ++--
 arch/x86/kernel/cpu/intel_rdt.h          | 2 +-
 arch/x86/kernel/cpu/intel_rdt_monitor.c  | 4 ++--
 arch/x86/kernel/cpu/intel_rdt_rdtgroup.c | 2 +-
 4 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c
index 97c8d83..b8dc141 100644
--- a/arch/x86/kernel/cpu/intel_rdt.c
+++ b/arch/x86/kernel/cpu/intel_rdt.c
@@ -447,7 +447,7 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d)
 
 	if (is_mbm_enabled()) {
 		INIT_DELAYED_WORK(&d->mbm_over, mbm_handle_overflow);
-		mbm_setup_overflow_handler(d);
+		mbm_setup_overflow_handler(d, MBM_OVERFLOW_INTERVAL);
 	}
 
 	return 0;
@@ -540,7 +540,7 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
 	} else if (r == &rdt_resources_all[RDT_RESOURCE_L3] &&
 		   cpu == d->mbm_work_cpu && is_mbm_enabled()) {
 		cancel_delayed_work(&d->mbm_over);
-		mbm_setup_overflow_handler(d);
+		mbm_setup_overflow_handler(d, 0);
 	}
 }
 
diff --git a/arch/x86/kernel/cpu/intel_rdt.h b/arch/x86/kernel/cpu/intel_rdt.h
index 4040bf1..3e48693 100644
--- a/arch/x86/kernel/cpu/intel_rdt.h
+++ b/arch/x86/kernel/cpu/intel_rdt.h
@@ -422,7 +422,7 @@ void mkdir_mondata_subdir_allrdtgrp(struct rdt_resource *r,
 				    struct rdt_domain *d);
 void mon_event_read(struct rmid_read *rr, struct rdt_domain *d,
 		    struct rdtgroup *rdtgrp, int evtid, int first);
-void mbm_setup_overflow_handler(struct rdt_domain *dom);
+void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms);
 void mbm_handle_overflow(struct work_struct *work);
 
 #endif /* _ASM_X86_INTEL_RDT_H */
diff --git a/arch/x86/kernel/cpu/intel_rdt_monitor.c b/arch/x86/kernel/cpu/intel_rdt_monitor.c
index d6bfdfd..8378785 100644
--- a/arch/x86/kernel/cpu/intel_rdt_monitor.c
+++ b/arch/x86/kernel/cpu/intel_rdt_monitor.c
@@ -417,9 +417,9 @@ out_unlock:
 	mutex_unlock(&rdtgroup_mutex);
 }
 
-void mbm_setup_overflow_handler(struct rdt_domain *dom)
+void mbm_setup_overflow_handler(struct rdt_domain *dom, unsigned long delay_ms)
 {
-	unsigned long delay = msecs_to_jiffies(MBM_OVERFLOW_INTERVAL);
+	unsigned long delay = msecs_to_jiffies(delay_ms);
 	int cpu;
 
 	if (!static_branch_likely(&rdt_enable_key))
diff --git a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
index 86a6979..b529f93 100644
--- a/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
+++ b/arch/x86/kernel/cpu/intel_rdt_rdtgroup.c
@@ -1140,7 +1140,7 @@ static struct dentry *rdt_mount(struct file_system_type *fs_type,
 	if (is_mbm_enabled()) {
 		r = &rdt_resources_all[RDT_RESOURCE_L3];
 		list_for_each_entry(dom, &r->domains, list)
-			mbm_setup_overflow_handler(dom);
+			mbm_setup_overflow_handler(dom, MBM_OVERFLOW_INTERVAL);
 	}
 
 	goto out;

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web