Re: [RFC PATCH 07/38] arm_mpam: resctrl: Add boilerplate cpuhp and domain allocation

[Ben Horgan <ben.horgan@arm.com>]

Hi Jonathan,

On 12/22/25 11:48, Jonathan Cameron wrote:
> 
>>>> +static struct mpam_resctrl_dom *
>>>> +mpam_resctrl_get_domain_from_cpu(int cpu, struct mpam_resctrl_res *res)
>>>> +{
>>>> +	struct mpam_resctrl_dom *dom;
>>>> +	struct rdt_ctrl_domain *ctrl_d;
>>>> +
>>>> +	lockdep_assert_cpus_held();
>>>> +
>>>> +	list_for_each_entry_rcu(ctrl_d, &res->resctrl_res.ctrl_domains,
>>>> +				hdr.list) {
>>>> +		dom = container_of(ctrl_d, struct mpam_resctrl_dom,
>>>> +				   resctrl_ctrl_dom);  
>>>
>>> I'm lazy so haven't checked for more code here in later patches, but
>>> if not, why not iterate the list to access the domain directly rather
>>> than jumping through the rdt_ctrl_domain?
>>>
>>> Something along lines of:
>>> 	
>>> 	list_for_each_entry_rcu(dom, &res->resctrl_res.ctrl_domains,
>>> 				resctrl_ctrl_dom.hdr.list) {
>>> 	}
>>>  
>>
>> Unless I've misunderstood I don't think this works because it's not what
>> the fs/resctrl code expects.
> 
> I think I explained this one badly.
> 
> This should be functionally identical to the above so no visible side
> effects outside of this code.  All this change is meant to do is wrap the
> container_of() in the list iterator. When using the _entry_ variants
> it is wrapping container_of() anyway so just going one level further
> up the hierarchy of nested structures.
> 
> struct a {
> 	struct b {
> 		struct list_head l;
> 	}
> }
> 
> It's actually a list of struct a as all elements on this list are
> struct b instances within struct a, but you are treating it as a list
> of struct b and then using a container_of() to get to struct a on
> each one.
> 
> The change is treat it as a list of struct a with the list_head happening
> to be wrapped in struct b.  Results in slightly simpler code and makes
> the point these are always struct a instances.

Thanks for the detailed explanation. This makes sense to me now and I'll
make the change.

> 
> Jonathan
> 
> 
> 
>>
>>
>>>> +
>>>> +		if (cpumask_test_cpu(cpu, &dom->ctrl_comp->affinity))
>>>> +			return dom;
>>>> +	}
>>>> +
>>>> +	return NULL;
>>>> +}
>>>> +
> 
> 

Thanks,

Ben