linux-kernel - Re: [PATCH v2 2/2] x86/resctrl: Enable user to view thread or core throttling mode

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] [day] [month] [year] [list]

Message-ID: <20200822090123.GA31906@zn.tnic>
Date:   Sat, 22 Aug 2020 11:01:23 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Fenghua Yu <fenghua.yu@...el.com>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Tony Luck <tony.luck@...el.com>,
        Reinette Chatre <reinette.chatre@...el.com>,
        Ravi V Shankar <ravi.v.shankar@...el.com>,
        Babu Moger <babu.moger@....com>, x86 <x86@...nel.org>,
        linux-kernel <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] x86/resctrl: Enable user to view thread or core
 throttling mode

On Fri, Aug 21, 2020 at 11:25:17AM -0700, Fenghua Yu wrote:
> Early Intel hardware implementations of Memory Bandwidth Allocation (MBA)
> could only control bandwidth at the processor core level. This meant that
> when two processes with different bandwidth allocations ran simultaneously
> on the same core the hardware had to resolve this difference. It did so by
> applying the higher throttling value (lower bandwidth) to both processes.
> 
> Newer implementations can apply different throttling values to each
> thread on a core.
> 
> Introduce a new resctrl file, "thread_throttle_mode", on Intel systems
> that shows to the user how throttling values are allocated, per-core or
> per-thread.
> 
> On systems that support per-core throttling, the file will display "max".
> On newer systems that support per-thread throttling, the file will display
> "per-thread".
> 
> AMD confirmed in
> https://lore.kernel.org/lkml/18d277fd-6523-319c-d560-66b63ff606b8@amd.com
> that AMD bandwidth allocation is already at thread level but that the AMD
> implementation does not use a memory delay throttle mode. So to avoid
> confusion the thread throttling mode would be UNDEFINED on AMD systems
> and the "thread_throttle_mode" file will not be visible.
> 
> Originally-by: Reinette Chatre <reinette.chatre@...el.com>
> Signed-off-by: Fenghua Yu <fenghua.yu@...el.com>
> Reviewed-by: Reinette Chatre <reinette.chatre@...el.com>
> ---
> Change log:
> v2:
> - Rename "arch_throttle_mode" to "throttle_mode" and
>   "rdtgroup_rftype_by_name()" to "rdtgroup_rftype_get_by_name()" (Boris).

But you didn't call it that. You called it:

get_rdtgroup_rftype_get_by_name

Two "get" and the prefix doesn't come first. So take this string
verbatim:

"rdtgroup_get_rftype_by_name"

and paste it everywhere where the function name is.

How about you slow down, do it right and then send it? It is not a
competition where the first to send, wins.

Also, pls redo the patches ontop of tip:x86/cache because there's James'
stuff there now and this one doesn't apply.

And test them again and *then* resend.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette