Messages in this thread | | | Date | Mon, 6 Feb 2023 15:17:55 -0800 | From | David Matlack <> | Subject | Re: [Patch v2 1/5] KVM: x86/mmu: Make separate function to check for SPTEs atomic write conditions |
| |
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@google.com>
Code looks fine, just grammar/writing nits above.
Reviewed-by: David Matlack <dmatlack@google.com>
| |