Re: [Patch v2 1/5] KVM: x86/mmu: Make separate function to check for SPTEs atomic write conditions
From: David Matlack
Date: Mon Feb 06 2023 - 18:18:05 EST
The shortlog is difficult to understand.
- I think it's more common to use "Add" or "Introduce" when talking
about adding a new function, rather than "Make".
- "atomic write conditions" does not mirror the code naming, which
checks for "volatile bits". e.g. The function is not called
kvm_tdp_mmu_spte_need_atomic_write().
"volatile bits" is, at this point, pretty standard terminology in KVM
MMU to refer to "bits that can change outside the MMU lock". So I would
suggest leaning on that here.
So something like this:
KVM: x86/mmu: Add helper function to check if an SPTE has volatile bits
On Fri, Feb 03, 2023 at 11:28:18AM -0800, Vipin Sharma wrote:
> Move condition checks in kvm_tdp_mmu_write_spte() for writing spte
> atomically in a separate function.
s/in a separate function/to a separate function/
>
> New function will be used inc
nit: Use complete sentences. e.g. "This new function ..." or just state
the name directly, e.g. "kvm_tdp_mmu_spte_has_volatile_bits() will be
used in ...".
> future commits to clear bits in SPTE.
s/to clear bits in SPTE/to optimize clearing bits in SPTEs/
>
> Signed-off-by: Vipin Sharma <vipinsh@xxxxxxxxxx>
Code looks fine, just grammar/writing nits above.
Reviewed-by: David Matlack <dmatlack@xxxxxxxxxx>