Messages in this thread |  | | Date | Wed, 3 Oct 2018 09:02:27 +0200 (CEST) | From | Thomas Gleixner <> | Subject | Re: [PATCH 2/3] x86/intel_rdt: CBM overlap should also check for overlap with CDP peer |
| |
On Wed, 26 Sep 2018, Reinette Chatre wrote: > /** > - * rdtgroup_cbm_overlaps - Does CBM for intended closid overlap with other > + * _rdtgroup_cbm_overlaps - Does CBM for intended closid overlap with other > * @r: Resource to which domain instance @d belongs. > * @d: The domain instance for which @closid is being tested. > * @cbm: Capacity bitmask being tested. > @@ -1049,8 +1048,8 @@ static int __attribute__((unused)) rdt_cdp_peer_get(struct rdt_resource *r, > * > * Return: false if CBM does not overlap, true if it does. > */ > -bool rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d, > - u32 _cbm, int closid, bool exclusive) > +static bool _rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d, > + u32 _cbm, int closid, bool exclusive)
Existing issue. The documentation uses @cbm, but the argument is _cbm.
Also please make this __rdtgroup_cbm_overlaps(). Double underscores are standing more out.
> { > unsigned long *cbm = (unsigned long *)&_cbm; > unsigned long *ctrl_b; > @@ -1087,6 +1086,44 @@ bool rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d, > return false; > } > > +/** > + * rdtgroup_cbm_overlaps - Does CBM overlap with other use of hardware > + * @r: Resource to which domain instance @d belongs. > + * @d: The domain instance for which @closid is being tested. > + * @cbm: Capacity bitmask being tested. > + * @closid: Intended closid for @cbm. > + * @exclusive: Only check if overlaps with exclusive resource groups > + * > + * Resources that can be allocated using a CBM can use the CBM to control > + * the overlap of these allocations. rdtgroup_cmb_overlaps() is the test > + * for overlap. Overlap test is not limited to the specific resource for > + * which the CBM is intended though - when dealing with CDP resources that > + * share the underlying hardware the overlap check should be performed on > + * the CDP resource sharing the hardware also. > + * > + * Refer to description of _rdtgroup_cbm_overlaps() for the details of the > + * overlap test. > + * > + * Return: true if CBM overlap detected, false if there is no overlap > + */ > +bool rdtgroup_cbm_overlaps(struct rdt_resource *r, struct rdt_domain *d, > + u32 _cbm, int closid, bool exclusive)
Ditto. And here is no reason for using _cbm.
> +{ > + struct rdt_resource *r_cdp; > + struct rdt_domain *d_cdp; > + bool ret; > + > + ret = _rdtgroup_cbm_overlaps(r, d, _cbm, closid, exclusive); > + if (ret) > + return ret;
if (__rdtgroup_cbm_overlaps(r, d, _cbm, closid, exclusive)) return true;
> + > + if (rdt_cdp_peer_get(r, d, &r_cdp, &d_cdp) == 0) > + return _rdtgroup_cbm_overlaps(r_cdp, d_cdp, _cbm, > + closid, exclusive);
if (rdt_cdp_peer_get(r, d, &r_cdp, &d_cdp) < 0) return false;
return __rdtgroup_cbm_overlaps(r_cpd, d_cdp, _cbm, closid, exclusive);
Makes the whole thing more obvious.
Thanks,
tglx
|  |