Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1710767
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH 3/3] x86/intel_rdt/cqm: Improve limbo list processing |
| Date | 2017-08-14 11:50 +0200 |
| Message-ID | <uekav-2tS-35@gated-at.bofh.it> (permalink) |
| References | <ucEdj-2Fa-7@gated-at.bofh.it> <ucEdj-2Fa-5@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Wed, 9 Aug 2017, Vikas Shivappa wrote:
> @@ -426,6 +426,9 @@ static int domain_setup_mon_state(struct rdt_resource *r, struct rdt_domain *d)
> GFP_KERNEL);
> if (!d->rmid_busy_llc)
> return -ENOMEM;
> + INIT_DELAYED_WORK(&d->cqm_limbo, cqm_handle_limbo);
> + if (has_busy_rmid(r, d))
> + cqm_setup_limbo_handler(d);
This is beyond silly. d->rmid_busy_llc is allocated a few lines above. How
would a bit be set here?
> }
> if (is_mbm_total_enabled()) {
> tsize = sizeof(*d->mbm_total);
> @@ -536,11 +539,25 @@ static void domain_remove_cpu(int cpu, struct rdt_resource *r)
> list_del(&d->list);
> if (is_mbm_enabled())
> cancel_delayed_work(&d->mbm_over);
> +
> + if (is_llc_occupancy_enabled() &&
> + has_busy_rmid(r, d))
What is that line break helping here and why can't you just unconditionally
cancel the work?
> + cancel_delayed_work(&d->cqm_limbo);
> +
> kfree(d);
> - } 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);
> + return;
> + }
> +
> + if (r == &rdt_resources_all[RDT_RESOURCE_L3]) {
> + if (is_mbm_enabled() && cpu == d->mbm_work_cpu) {
> + cancel_delayed_work(&d->mbm_over);
> + mbm_setup_overflow_handler(d);
I think this is the wrong approach. If the timer is about to fire you
essentially double the interval. So you better flush the work, which will
reschedule it if needed.
> + }
> + if (is_llc_occupancy_enabled() && cpu == d->mbm_work_cpu &&
That want's to be d->cbm_work_cpu, right?
> + has_busy_rmid(r, d)) {
> + cancel_delayed_work(&d->cqm_limbo);
> + cqm_setup_limbo_handler(d);
See above.
Thanks,
tglx
Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread
Re: [PATCH 3/3] x86/intel_rdt/cqm: Improve limbo list processing Thomas Gleixner <tglx@linutronix.de> - 2017-08-14 11:50 +0200
Re: [PATCH 3/3] x86/intel_rdt/cqm: Improve limbo list processing Shivappa Vikas <vikas.shivappa@intel.com> - 2017-08-14 21:20 +0200
Re: [PATCH 3/3] x86/intel_rdt/cqm: Improve limbo list processing Shivappa Vikas <vikas.shivappa@intel.com> - 2017-08-14 23:20 +0200
csiph-web