linux-kernel - Re: [PATCH v5 3/8] x86/resctrl: Split the rdt

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 27 Sep 2023 08:22:38 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     "Luck, Tony" <tony.luck@...el.com>
CC:     "Yu, Fenghua" <fenghua.yu@...el.com>,
        Peter Newman <peternewman@...gle.com>,
        Jonathan Corbet <corbet@....net>,
        Shuah Khan <skhan@...uxfoundation.org>,
        "x86@...nel.org" <x86@...nel.org>,
        Shaopeng Tan <tan.shaopeng@...itsu.com>,
        James Morse <james.morse@....com>,
        Jamie Iles <quic_jiles@...cinc.com>,
        Babu Moger <babu.moger@....com>,
        Randy Dunlap <rdunlap@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "patches@...ts.linux.dev" <patches@...ts.linux.dev>
Subject: Re: [PATCH v5 3/8] x86/resctrl: Split the rdt_domain structure

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