linux-kernel - RE: [PATCH] x86/resctrl: mba_MBps: Fall back to total b/w if local b/w unavailable

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]

Message-ID: <SJ1PR11MB60837D379853EFB14A6A20BBFCDDA@SJ1PR11MB6083.namprd11.prod.outlook.com>
Date:   Thu, 26 Oct 2023 16:09:19 +0000
From:   "Luck, Tony" <tony.luck@...el.com>
To:     "babu.moger@....com" <babu.moger@....com>,
        Peter Newman <peternewman@...gle.com>
CC:     "Yu, Fenghua" <fenghua.yu@...el.com>,
        "Chatre, Reinette" <reinette.chatre@...el.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>,
        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] x86/resctrl: mba_MBps: Fall back to total b/w if local
 b/w unavailable

> > What I meant was I think it would be enough to just give the function
> > you added a name that's more specific to the Mbps controller use case.
> > For example, get_mba_sc_mbm_state().
>
> I actually liked this idea. Add a new function get_mba_sc_mbm_state. That
> way we exactly know why this function is used. I see you already sent a v2
> making the event global. Making it global may not be good idea. Can you
> please update the patch and resend. Also please add the comment about why
> you are adding that function.

Can you explain why you don't like the global? If there is a better name for it,
or a better comment for what it does, or you think the code that sets the value
could be clearer, then I'm happy to make changes there.

Which events are supported by a system is a static property. Figuring out once
at "init" time which event to use for mba_MBps seems a better choice than
re-checking for each of possibly hundreds of RMIDs every second. Even though
the check is cheap, it is utterly pointless.

-Tony