Messages in this thread | | | Date | Thu, 12 Jan 2023 12:12:02 +0100 | From | Borislav Petkov <> | Subject | Re: [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
| |