lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Thu, 18 Apr 2019 15:03:35 +0800 From: Xiaochen Shen <xiaochen.shen@...el.com> To: tony.luck@...el.com, x86@...nel.org, mingo@...hat.com, bp@...e.de, reinette.chatre@...el.com, hpa@...or.com, tglx@...utronix.de, mingo@...nel.org, linux-kernel@...r.kernel.org, fenghua.yu@...el.com, linux-tip-commits@...r.kernel.org Cc: Xiaochen Shen <xiaochen.shen@...el.com> Subject: Re: [tip:x86/cache] x86/resctrl: Initialize a new resource group with default MBA values Hi Boris, I found a nitpick - an unnecessary newline at the end of the patch. Please help double check. Thank you. On 4/18/2019 6:14, tip-bot for Xiaochen Shen wrote: > Commit-ID: 47820e73f5b3a1fdb8ebd1219191edc96e0c85c1 > Gitweb: https://git.kernel.org/tip/47820e73f5b3a1fdb8ebd1219191edc96e0c85c1 > Author: Xiaochen Shen <xiaochen.shen@...el.com> > AuthorDate: Wed, 17 Apr 2019 19:08:49 +0800 > Committer: Borislav Petkov <bp@...e.de> > CommitDate: Thu, 18 Apr 2019 00:06:31 +0200 > > x86/resctrl: Initialize a new resource group with default MBA values > > Currently, when a new resource group is created, the allocation values > of the MBA resource are not initialized and remain meaningless data. > > For example: > > mkdir /sys/fs/resctrl/p1 > cat /sys/fs/resctrl/p1/schemata > MB:0=100;1=100 > > echo "MB:0=10;1=20" > /sys/fs/resctrl/p1/schemata > cat /sys/fs/resctrl/p1/schemata > MB:0= 10;1= 20 > > rmdir /sys/fs/resctrl/p1 > mkdir /sys/fs/resctrl/p2 > cat /sys/fs/resctrl/p2/schemata > MB:0= 10;1= 20 > > Therefore, when the new group is created, it is reasonable to initialize > MBA resource with default values. > > Initialize the MBA resource and cache resources in separate functions. > > [ bp: Add newlines between code blocks for better readability. ] > > Signed-off-by: Xiaochen Shen <xiaochen.shen@...el.com> > Signed-off-by: Borislav Petkov <bp@...e.de> > Reviewed-by: Fenghua Yu <fenghua.yu@...el.com> > Reviewed-by: Reinette Chatre <reinette.chatre@...el.com> > Cc: "H. Peter Anvin" <hpa@...or.com> > Cc: Ingo Molnar <mingo@...hat.com> > Cc: pei.p.jia@...el.com > Cc: Thomas Gleixner <tglx@...utronix.de> > Cc: Tony Luck <tony.luck@...el.com> > Cc: x86-ml <x86@...nel.org> > Link: https://lkml.kernel.org/r/1555499329-1170-3-git-send-email-xiaochen.shen@intel.com > --- > arch/x86/kernel/cpu/resctrl/ctrlmondata.c | 4 +-- > arch/x86/kernel/cpu/resctrl/rdtgroup.c | 52 ++++++++++++++++++++----------- > 2 files changed, 35 insertions(+), 21 deletions(-) > > diff --git a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c > index 2dbd990a2eb7..89320c0396b1 100644 > --- a/arch/x86/kernel/cpu/resctrl/ctrlmondata.c > +++ b/arch/x86/kernel/cpu/resctrl/ctrlmondata.c > @@ -342,10 +342,10 @@ int update_domains(struct rdt_resource *r, int closid) > if (cpumask_empty(cpu_mask) || mba_sc) > goto done; > cpu = get_cpu(); > - /* Update CBM on this cpu if it's in cpu_mask. */ > + /* Update resource control msr on this CPU if it's in cpu_mask. */ > if (cpumask_test_cpu(cpu, cpu_mask)) > rdt_ctrl_update(&msr_param); > - /* Update CBM on other cpus. */ > + /* Update resource control msr on other CPUs. */ > smp_call_function_many(cpu_mask, rdt_ctrl_update, &msr_param, 1); > put_cpu(); > > diff --git a/arch/x86/kernel/cpu/resctrl/rdtgroup.c b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > index 36ace51ee705..333c177a2471 100644 > --- a/arch/x86/kernel/cpu/resctrl/rdtgroup.c > +++ b/arch/x86/kernel/cpu/resctrl/rdtgroup.c > @@ -2581,8 +2581,8 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r, > return 0; > } > > -/** > - * rdtgroup_init_alloc - Initialize the new RDT group's allocations > +/* > + * Initialize cache resources with default values. > * > * A new RDT group is being created on an allocation capable (CAT) > * supporting system. Set this group up to start off with all usable > @@ -2591,38 +2591,52 @@ static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r, > * If there are no more shareable bits available on any domain then > * the entire allocation will fail. > */ > +static int rdtgroup_init_cat(struct rdt_resource *r, u32 closid) > +{ > + struct rdt_domain *d; > + int ret; > + > + list_for_each_entry(d, &r->domains, list) { > + ret = __init_one_rdt_domain(d, r, closid); > + if (ret < 0) > + return ret; > + } > + > + return 0; > +} > + > +/* Initialize MBA resource with default values. */ > +static void rdtgroup_init_mba(struct rdt_resource *r) > +{ > + struct rdt_domain *d; > + > + list_for_each_entry(d, &r->domains, list) { > + d->new_ctrl = is_mba_sc(r) ? MBA_MAX_MBPS : r->default_ctrl; > + d->have_new_ctrl = true; > + } > +} > + > +/* Initialize the RDT group's allocations. */ > static int rdtgroup_init_alloc(struct rdtgroup *rdtgrp) > { > struct rdt_resource *r; > - struct rdt_domain *d; > int ret; > > for_each_alloc_enabled_rdt_resource(r) { > - /* > - * Only initialize default allocations for CBM cache > - * resources > - */ > - if (r->rid == RDT_RESOURCE_MBA) > - continue; > - list_for_each_entry(d, &r->domains, list) { > - ret = __init_one_rdt_domain(d, r, rdtgrp->closid); > + if (r->rid == RDT_RESOURCE_MBA) { > + rdtgroup_init_mba(r); > + } else { > + ret = rdtgroup_init_cat(r, rdtgrp->closid); > if (ret < 0) > return ret; > } > - } > > - for_each_alloc_enabled_rdt_resource(r) { > - /* > - * Only initialize default allocations for CBM cache > - * resources > - */ > - if (r->rid == RDT_RESOURCE_MBA) > - continue; > ret = update_domains(r, rdtgrp->closid); > if (ret < 0) { > rdt_last_cmd_puts("Failed to initialize allocations\n"); > return ret; > } > + In my opinion, this newline is unnecessary. Thank you. > } > > rdtgrp->mode = RDT_MODE_SHAREABLE; > -- Best regards, Xiaochen
Powered by blists - more mailing lists