Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1502056
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology |
| Date | 2016-10-17 16:50 +0200 |
| Message-ID | <sthoK-4JB-25@gated-at.bofh.it> (permalink) |
| References | <ssjVD-7Za-5@gated-at.bofh.it> <ssjVE-7Za-31@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On Fri, 14 Oct 2016, Fenghua Yu wrote:
>
> +static int get_cache_id(int cpu, int level)
> +{
> + struct cpu_cacheinfo *ci = get_cpu_cacheinfo(cpu);
> + int i;
> +
> + for (i = 0; i < ci->num_leaves; i++)
> + if (ci->info_list[i].level == level)
> + return ci->info_list[i].id;
Multi line statements need curly braces.
> + return -1;
> +}
> +
> +void rdt_cbm_update(void *arg)
> +{
> + struct msr_param *m = (struct msr_param *)arg;
> + struct rdt_resource *r = m->res;
> + struct rdt_domain *d;
> + struct list_head *l;
> + int i, cpu = smp_processor_id();
> +
> + list_for_each(l, &r->domains) {
> + d = list_entry(l, struct rdt_domain, list);
We have list_for_each_entry() ....
> + if (cpumask_test_cpu(cpu, &d->cpu_mask))
> + goto found;
> + }
> + pr_info_once("cpu %d not found in any domain for resource %s\n",
> + cpu, r->name);
If the list is empty, then 'd' is not initialized and dereferenced. If the
list is not empty then you use the last domain in the list. Neither one is
the right thing to do....
> +
> +found:
> + for (i = m->low; i < m->high; i++)
> + wrmsrl(r->msr_base + i, d->cbm[i]);
> +}
> +
> +static void update_domain(int cpu, struct rdt_resource *r, int add)
> +{
> + struct list_head *l;
> + struct rdt_domain *d;
> + int i, cache_id;
+ struct rdt_domain *d;
+ struct list_head *l;
+ int i, cache_id;
Can you see how this is simpler to read?
> +
> + cache_id = get_cache_id(cpu, r->cache_level);
> + if (cache_id == -1) {
> + pr_info_once("Could't find cache id for cpu %d\n", cpu);
> + return;
> + }
Please seperate this by a empty line.
> + list_for_each(l, &r->domains) {
> + d = list_entry(l, struct rdt_domain, list);
list_for_each_entry() once more.
> + if (cache_id == d->id)
> + goto found;
> + if (cache_id < d->id)
> + break;
> + }
> + if (!add) {
> + pr_info_once("removed unknown cpu %d\n", cpu);
_once? Why should this happen at all?
> + return;
> + }
> + d = kzalloc(sizeof(*d), GFP_KERNEL);
Shouldn't this be kzalloc_node() ?
> + if (!d)
> + return;
> +
> + d->id = cache_id;
> + d->cbm = kmalloc_array(r->max_closid, sizeof(*d->cbm), GFP_KERNEL);
> + if (!d->cbm) {
> + pr_info("Failed to alloc CBM array for cpu %d\n", cpu);
> + kfree(d);
> + return;
> + }
> + cpumask_set_cpu(cpu, &d->cpu_mask);
> + for (i = 0; i < r->max_closid; i++) {
> + d->cbm[i] = r->max_cbm;
> + wrmsrl(r->msr_base + i, d->cbm[i]);
> + }
> + list_add_tail(&d->list, l);
> + r->num_domains++;
> + return;
> +
> +found:
> + if (add) {
> + cpumask_set_cpu(cpu, &d->cpu_mask);
Gah. Reusing this function for add and del is just a silly optimization
which makes the code harder to read.
All you share is the find thing. So you can split that out into a helper:
static struct rdt_domain *rdt_find_domain(struct rdt_resource *r,
unsigned int cpu, int id)
{
if (id < 0)
return ERR_PTR(id);
list_for_each_entry(d, &r->domains, list) {
if (d->id == cache_id)
return d;
}
return NULL;
}
and use it for seperate add/del functions.
static void domain_add_cpu(unsigned int cpu, struct rdt_resource *r)
{
int id = get_cache_id(cpu, r->cache_level);
struct rdt_domain *d;
d = rdt_find_domain(r, cpu, id);
if (IS_ERR(d)) {
pr_warn("Print some useful information\n", r->cache_level, cpu, ....);
return;
}
if (d) {
cpumask_set_cpu(cpu, &d->cpu_mask);
return;
}
/* Do the allocaction stuff */
}
static void domain_remove_cpu(unsigned int cpu, struct rdt_resource *r)
{
int id = get_cache_id(cpu, r->cache_level);
struct rdt_domain *d;
d = rdt_find_domain(r, cpu, id);
if (IS_ERR_OR_NULL(d)) {
pr_warn("Print some useful information\n", r->cache_level, cpu, ...);
return;
}
cpumask_clear_cpu(cpu, &d->cpu_mask);
....
}
Hmm?
> static int __init intel_rdt_late_init(void)
> {
> struct rdt_resource *r;
> + int state;
>
> if (!get_rdt_resources())
> return -ENODEV;
>
> + state = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> + "AP_INTEL_RDT_ONLINE",
Please use: "x86/rdt/cat:online:" or similar. We messed that up in a few
places when adding the names, but we don't want to add more of this.
> + intel_rdt_online_cpu, intel_rdt_offline_cpu);
Thanks,
tglx
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v4 00/18] Intel Cache Allocation Technology "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
[PATCH v4 02/18] cacheinfo: Introduce cache id "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 02/18] cacheinfo: Introduce cache id Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 12:40 +0200
[PATCH v4 16/18] x86/intel_rdt: Add schemata file "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 16/18] x86/intel_rdt: Add schemata file Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 00:40 +0200
[PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 23:20 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 00:00 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:00 +0200
RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:10 +0200
RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:20 +0200
RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:30 +0200
RE: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:10 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Fenghua Yu <fenghua.yu@intel.com> - 2016-10-18 00:20 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 01:30 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 01:40 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Fenghua Yu <fenghua.yu@intel.com> - 2016-10-18 02:00 +0200
Re: [PATCH v4 13/18] x86/intel_rdt: Add mkdir to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 12:50 +0200
[PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 15:50 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Fenghua Yu <fenghua.yu@intel.com> - 2016-10-17 17:10 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID "Luck, Tony" <tony.luck@intel.com> - 2016-10-17 18:40 +0200
RE: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID "Yu, Fenghua" <fenghua.yu@intel.com> - 2016-10-17 18:50 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID "Luck, Tony" <tony.luck@intel.com> - 2016-10-17 22:30 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 19:10 +0200
Re: [PATCH v4 08/18] x86/intel_rdt: Pick up L3/L2 RDT parameters from CPUID Fenghua Yu <fenghua.yu@intel.com> - 2016-10-17 20:20 +0200
[PATCH v4 01/18] Documentation, ABI: Add a document entry for cache id "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 01/18] Documentation, ABI: Add a document entry for cache id Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 12:40 +0200
[PATCH v4 18/18] MAINTAINERS: Add maintainer for Intel RDT resource allocation "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
[PATCH v4 04/18] x86/intel_rdt: Feature discovery "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
[PATCH v4 11/18] x86/intel_rdt: Add basic resctrl filesystem support "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 11/18] x86/intel_rdt: Add basic resctrl filesystem support Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 21:40 +0200
[PATCH v4 05/18] Documentation, x86: Documentation for Intel resource allocation user interface "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
[PATCH v4 17/18] x86/intel_rdt: Add scheduler hook "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
[PATCH v4 14/18] x86/intel_rdt: Add cpus file "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 14/18] x86/intel_rdt: Add cpus file Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 23:40 +0200
[PATCH v4 12/18] x86/intel_rdt: Add "info" files to resctrl file system "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 12/18] x86/intel_rdt: Add "info" files to resctrl file system Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 21:50 +0200
[PATCH v4 15/18] x86/intel_rdt: Add tasks files "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files Thomas Gleixner <tglx@linutronix.de> - 2016-10-18 00:10 +0200
Re: [PATCH v4 15/18] x86/intel_rdt: Add tasks files "Luck, Tony" <tony.luck@intel.com> - 2016-10-18 00:20 +0200
[PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 10/18] x86/intel_rdt: Build structures for each resource based on cache topology Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 16:50 +0200
[PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86 "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 03/18] x86, intel_cacheinfo: Enable cache id in x86 Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 13:00 +0200
[PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery "Fenghua Yu" <fenghua.yu@intel.com> - 2016-10-15 01:20 +0200
Re: [PATCH v4 07/18] x86/intel_rdt: Add Haswell feature discovery Thomas Gleixner <tglx@linutronix.de> - 2016-10-17 13:10 +0200
csiph-web