Messages in this thread | | | Date | Wed, 4 Nov 2015 09:55:57 -0500 | From | Luiz Capitulino <> | Subject | Re: [PATCH V15 05/11] x86/intel_rdt: Add Class of service management |
| |
On Thu, 1 Oct 2015 23:09:39 -0700 Fenghua Yu <fenghua.yu@intel.com> wrote:
> Adds some data-structures and APIs to support Class of service > management(closid). There is a new clos_cbm table which keeps a 1:1 > mapping between closid and capacity bit mask (cbm) > and a count of usage of closid. Each task would be associated with a > Closid at a time and this patch adds a new field closid to task_struct > to keep track of the same. > > Signed-off-by: Vikas Shivappa <vikas.shivappa@linux.intel.com> > Signed-off-by: Fenghua Yu <fenghua.yu@intel.com> > --- > arch/x86/include/asm/intel_rdt.h | 12 ++++++ > arch/x86/kernel/cpu/intel_rdt.c | 82 +++++++++++++++++++++++++++++++++++++++- > include/linux/sched.h | 3 ++ > 3 files changed, 95 insertions(+), 2 deletions(-) > create mode 100644 arch/x86/include/asm/intel_rdt.h > > diff --git a/arch/x86/include/asm/intel_rdt.h b/arch/x86/include/asm/intel_rdt.h > new file mode 100644 > index 0000000..88b7643 > --- /dev/null > +++ b/arch/x86/include/asm/intel_rdt.h > @@ -0,0 +1,12 @@ > +#ifndef _RDT_H_ > +#define _RDT_H_ > + > +#ifdef CONFIG_INTEL_RDT > + > +struct clos_cbm_table { > + unsigned long l3_cbm; > + unsigned int clos_refcnt; > +};
Isn't this a single entry?
> + > +#endif > +#endif > diff --git a/arch/x86/kernel/cpu/intel_rdt.c b/arch/x86/kernel/cpu/intel_rdt.c > index f49e970..d79213a 100644 > --- a/arch/x86/kernel/cpu/intel_rdt.c > +++ b/arch/x86/kernel/cpu/intel_rdt.c > @@ -24,17 +24,95 @@ > > #include <linux/slab.h> > #include <linux/err.h> > +#include <asm/intel_rdt.h> > + > +/* > + * cctable maintains 1:1 mapping between CLOSid and cache bitmask. > + */ > +static struct clos_cbm_table *cctable; > +/* > + * closid availability bit map. > + */ > +unsigned long *closmap; > +static DEFINE_MUTEX(rdt_group_mutex); > + > +static inline void closid_get(u32 closid) > +{ > + struct clos_cbm_table *cct = &cctable[closid]; > + > + lockdep_assert_held(&rdt_group_mutex); > + > + cct->clos_refcnt++; > +} > + > +static int closid_alloc(u32 *closid) > +{ > + u32 maxid; > + u32 id; > + > + lockdep_assert_held(&rdt_group_mutex); > + > + maxid = boot_cpu_data.x86_cache_max_closid; > + id = find_first_zero_bit(closmap, maxid); > + if (id == maxid) > + return -ENOSPC;
This causes echo to print "No space left in device" when you run out of CBMs. I think -ENOMEM makes more sense.
> + > + set_bit(id, closmap); > + closid_get(id); > + *closid = id; > + > + return 0; > +} > + > +static inline void closid_free(u32 closid) > +{ > + clear_bit(closid, closmap); > + cctable[closid].l3_cbm = 0; > +} > + > +static void closid_put(u32 closid) > +{ > + struct clos_cbm_table *cct = &cctable[closid]; > + > + lockdep_assert_held(&rdt_group_mutex); > + if (WARN_ON(!cct->clos_refcnt)) > + return; > + > + if (!--cct->clos_refcnt) > + closid_free(closid); > +}
This is very minor, but IMHO you got closid_put() and closid_free() naming backwards. closeid_free() is the opposite operation of closeid_alloc(). Likewise, closeid_put() is the opposite of closide_get().
> > static int __init intel_rdt_late_init(void) > { > struct cpuinfo_x86 *c = &boot_cpu_data; > + u32 maxid, max_cbm_len; > + int err = 0, size; > > if (!cpu_has(c, X86_FEATURE_CAT_L3)) > return -ENODEV; > > - pr_info("Intel cache allocation detected\n"); > + maxid = c->x86_cache_max_closid; > + max_cbm_len = c->x86_cache_max_cbm_len; > > - return 0; > + size = maxid * sizeof(struct clos_cbm_table); > + cctable = kzalloc(size, GFP_KERNEL); > + if (!cctable) { > + err = -ENOMEM; > + goto out_err; > + } > + > + size = BITS_TO_LONGS(maxid) * sizeof(long); > + closmap = kzalloc(size, GFP_KERNEL); > + if (!closmap) { > + kfree(cctable); > + err = -ENOMEM; > + goto out_err; > + } > + > + pr_info("Intel cache allocation enabled\n"); > +out_err: > + > + return err; > } > > late_initcall(intel_rdt_late_init); > diff --git a/include/linux/sched.h b/include/linux/sched.h > index b7b9501..24bfbac 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1668,6 +1668,9 @@ 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
| |