Re: [PATCH 1/3] x86/intel_rdt: Introduce utility to obtain CDP peer

[Thomas Gleixner]

On Wed, 26 Sep 2018, Reinette Chatre wrote:
> + * Return: 0 if a CDP peer was found, <0 on error or if no CDP peer exists.
> + *         If a CDP peer was found, @r_cdp will point to the peer RDT resource
> + *         and @d_cdp will point to the peer RDT domain.
> + */
> +static int __attribute__((unused)) rdt_cdp_peer_get(struct rdt_resource *r,
> +						    struct rdt_domain *d,
> +						    struct rdt_resource **r_cdp,
> +						    struct rdt_domain **d_cdp)
> +{
> +	struct rdt_resource *_r_cdp = NULL;
> +	struct rdt_domain *_d_cdp = NULL;
> +	int ret = 0;
> +
> +	switch (r->rid) {
> +	case RDT_RESOURCE_L3DATA:
> +		_r_cdp = &rdt_resources_all[RDT_RESOURCE_L3CODE];
> +		break;
> +	case RDT_RESOURCE_L3CODE:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L3DATA];
> +		break;
> +	case RDT_RESOURCE_L2DATA:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L2CODE];
> +		break;
> +	case RDT_RESOURCE_L2CODE:
> +		_r_cdp =  &rdt_resources_all[RDT_RESOURCE_L2DATA];
> +		break;
> +	default:
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	/*
> +	 * When a new CPU comes online and CDP is enabled then the new
> +	 * RDT domains (if any) associated with both CDP RDT resources
> +	 * are added in the same CPU online routine while the
> +	 * rdtgroup_mutex is held. It should thus not happen for one
> +	 * RDT domain to exist and be associated with its RDT CDP
> +	 * resource but there is no RDT domain associated with the
> +	 * peer RDT CDP resource. Hence the WARN.
> +	 */
> +	_d_cdp = rdt_find_domain(_r_cdp, d->id, NULL);
> +	if (WARN_ON(!_d_cdp)) {
> +		_r_cdp = NULL;
> +		ret = -ENOENT;

While this should never happen, the return value is ambiguous. I'd rather
use EINVAL or such and propagate it further down at the call site.

Thanks,

	tglx