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: Wed, 1 Mar 2017 15:11:20 +0100 (CET) From: Thomas Gleixner <tglx@...utronix.de> To: Vikas Shivappa <vikas.shivappa@...ux.intel.com> cc: vikas.shivappa@...el.com, linux-kernel@...r.kernel.org, x86@...nel.org, hpa@...or.com, mingo@...nel.org, peterz@...radead.org, ravi.v.shankar@...el.com, tony.luck@...el.com, fenghua.yu@...el.com, andi.kleen@...el.com Subject: Re: [PATCH 4/5] x86/intel_rdt: Reset the cbm MSR during rmdir On Fri, 17 Feb 2017, Vikas Shivappa wrote: > During rmdir reset the ctrl values to all 1s in the QOS_MSR for the > directory's closid. This is done so that that next time when the closid > is reused they dont reflect old values. Sigh. > +static int reset_all_ctrls(struct rdt_resource *r, u32 sclosid, u32 eclosid) What's so wrong with using 'start' and 'end' instead of these cryptic names? > { > struct msr_param msr_param; > cpumask_var_t cpu_mask; > @@ -791,8 +791,8 @@ static int reset_all_cbms(struct rdt_resource *r) > return -ENOMEM; > > msr_param.res = r; > - msr_param.low = 0; > - msr_param.high = r->num_closid; > + msr_param.low = sclosid; > + msr_param.high = eclosid; > > /* > * Disable resource control for this resource by setting all > @@ -802,7 +802,7 @@ static int reset_all_cbms(struct rdt_resource *r) > list_for_each_entry(d, &r->domains, list) { > cpumask_set_cpu(cpumask_any(&d->cpu_mask), cpu_mask); > > - for (i = 0; i < r->num_closid; i++) > + for (i = sclosid; i < eclosid; i++) 'eclosid' or even when named 'end' is an outright bogus argument to this function. What you really want is: static int reset_all_ctrls(struct rdt_resource *r, u32 closid, u32 count) which works here: > for_each_enabled_rdt_resource(r) > - reset_all_cbms(r); > + reset_all_ctrls(r, 0, r->num_closid); and makes this part understandable: > + /* > + * Put domain control values back to default for the > + * rdtgrp thats being removed. > + */ > + for_each_enabled_rdt_resource(r) > + reset_all_ctrls(r, rdtgrp->closid, rdtgrp->closid + 1); reset_all_ctrls(r, rdtgrp->closid, 1); What you have done: > + reset_all_ctrls(r, rdtgrp->closid, rdtgrp->closid + 1); made me really look twice to figure out what the heck this is about. Thanks, tglx
Powered by blists - more mailing lists