linux-kernel - Re: [PATCH v2 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters

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: <Z5AKFlSJ9wgbX_MS@google.com>
Date: Tue, 21 Jan 2025 12:56:54 -0800
From: Sean Christopherson <seanjc@...gle.com>
To: Colton Lewis <coltonlewis@...gle.com>
Cc: kvm@...r.kernel.org, mizhang@...gle.com, ljr.kernel@...il.com, 
	jmattson@...gle.com, aaronlewis@...gle.com, pbonzini@...hat.com, 
	shuah@...nel.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/6] KVM: x86: selftests: Set up AMD VM in pmu_counters_test

On Mon, Jan 20, 2025, Colton Lewis wrote:
> > > +static void test_core_counters(void)
> > > +{
> > > +	uint8_t nr_counters = nr_core_counters();
> > > +	bool core_ext = kvm_cpu_has(X86_FEATURE_PERF_CTR_EXT_CORE);
> > > +	bool perfmon_v2 = kvm_cpu_has(X86_FEATURE_PERFMON_V2);
> > > +	struct kvm_vcpu *vcpu;
> > > +	struct kvm_vm *vm;
> > > +
> > > +	for (uint8_t ce = 0; ce <= core_ext; ce++) {
> 
> > Kernel style is to not declared variables inside for-loops.
> 
> I ran it through checkpatch and it didn't complain.

...

> > > +					vcpu_set_cpuid_property(
> 
> > Google3!  (Never, ever wrap immediately after the opening paranethesis).
> 
> Checkpatch didn't complain.

Checkpatch is a perl script, not sentient AI.  It's nothing more than a tool to
help detect common goofs, typos, egregious flaws, etc.  The absense of checkpatch
warnings/errors does not mean a patch has no issues.  Coding style in particular
is quite subjective and prone to "exceptions to the rule", which makes is especially
hard to "enforce" via checkpatch.

As explained in Documentation/process/4.Coding.rst, what matters most is consistency:

  A code base as large as the kernel requires some uniformity of code to make it
  possible for developers to quickly understand any part of it.  So there is no
  longer room for strangely-formatted code.

https://www.kernel.org/doc/html/v5.0/process/4.Coding.html#coding-style