lkml.org 
[lkml]   [2023]   [Jan]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH kernel v2 1/3] x86/amd: Cache values in percpu variables
On Thu, Jan 12, 2023 at 04:21:28PM +1100, Alexey Kardashevskiy wrote:
> "that function" is set_dr_addr_mask() (btw should I rename it to start with
> amd_?

If you really wanna... I mean the code is already doing AMD-specific handling
but sure, it'll be more obvious then that arch_install_hw_breakpoint() does only
AMD-specific masking there under the info->mask test.

> If it is out of bounds, it won't neither set or get. And these 2 helpers are
> used only by the kernel and the callers pass 0..3 and nothing else. BUG_ON()
> would do too, for example.

Yeah, we don't do BUG_ON - look for Linus' colorful explanations why. :)

In any case, I think we should always aim for proper recovery from errors but
this case is not that important so let's leave it at the WARN_ON_ONCE.

> imho having 1,2,3 in the code eliminates the need in a comment and produces
> the exact same end result. But since nobody cares, I'll do it the shorter
> way with just +1 and +2.

Yeah, the more important goal is simplicity. And that pays off when you have to
revisit that code and figure out what it does, later.

> Sure. Out of curiosity - why?^w^w^w^w^ it reduced the vmlinux size by 48
> bytes, nice.

The same answer - simplicity and speed when reading it.

That

if (per_cpu(amd_dr_addr_mask, smp_processor_id())[dr] == mask)

is a bit harder to parse than

if (per_cpu(amd_dr_addr_mask, cpu)[dr] == mask)

Thx.

--
Regards/Gruss,
Boris.

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

\
 
 \ /
  Last update: 2023-03-26 23:37    [W:0.074 / U:0.012 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site