Re: [PATCH v5 3/8] x86/resctrl: Split the rdt_domain structure

[Reinette Chatre]

Hi Tony,

On 9/26/2023 5:00 PM, Luck, Tony wrote:
>> I expect you are right and your proposal makes the code cleaner.
>> Could the list_entry() call within rdt_find_domain() instead
>> extract a pointer to a struct rdt_domain_hdr? To me that would make
>> it most generic and avoid using wrong type for a pointer. I do think that
>> may have been what you intended by moving id in there though ...
> 
> Reinette,
> 
> Exactly my thoughts! The function currently looks like this
> (though I expect my mail client may mess up the formatting):
> 
> struct rdt_domain_hdr *rdt_find_domain(struct list_head *h, int id,
>                                        struct list_head **pos)
> {
>         struct rdt_domain_hdr *d;
>         struct list_head *l;
> 
>         if (id < 0)
>                 return ERR_PTR(-ENODEV);
> 
>         list_for_each(l, h) {
>                 d = list_entry(l, struct rdt_domain_hdr, list);
>                 /* When id is found, return its domain. */
>                 if (id == d->id)
>                         return d;
>                 /* Stop searching when finding id's position in sorted list. */
>                 if (id < d->id)
>                         break;
>         }
> 
>         if (pos)
>                 *pos = l;
> 
>         return NULL;
> }
> 
> and a typical caller does this:
> 
>         struct rdt_domain_hdr *hdr;
>         struct rdt_mon_domain *d;
> 
> 
> 
> 
>         hdr = rdt_find_domain(&r->mon_domains, id, &add_pos);
>         if (IS_ERR(hdr)) {
>                 pr_warn("Couldn't find monitor scope id=%d for CPU %d\n", id, cpu);
>                 return;
>         }
>         d = container_of(hdr, struct rdt_mon_domain, hdr);
> 

This looks very good to me. Thank you very much for creating a
solution that integrates well.

Reinette