All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Matlack <dmatlack@google.com>
To: Vipin Sharma <vipinsh@google.com>
Cc: seanjc@google.com, pbonzini@redhat.com, bgardon@google.com,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [Patch v2 1/5] KVM: x86/mmu: Make separate function to check for SPTEs atomic write conditions
Date: Mon, 6 Feb 2023 15:17:55 -0800	[thread overview]
Message-ID: <Y+GKo0tI2NeKiNDR@google.com> (raw)
In-Reply-To: <20230203192822.106773-2-vipinsh@google.com>

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>

  parent reply	other threads:[~2023-02-06 23:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-03 19:28 [Patch v2 0/5] Optimize clear dirty log Vipin Sharma
2023-02-03 19:28 ` [Patch v2 1/5] KVM: x86/mmu: Make separate function to check for SPTEs atomic write conditions Vipin Sharma
2023-02-06 22:09   ` Ben Gardon
2023-02-06 23:17   ` David Matlack [this message]
2023-02-03 19:28 ` [Patch v2 2/5] KVM: x86/mmu: Optimize SPTE change flow for clear-dirty-log Vipin Sharma
2023-02-06 22:06   ` Ben Gardon
2023-02-06 23:34     ` David Matlack
2023-02-07 17:29     ` Vipin Sharma
2023-02-06 23:41   ` David Matlack
2023-02-07 17:36     ` Vipin Sharma
2023-02-07 17:47       ` David Matlack
2023-02-08 23:44         ` Vipin Sharma
2023-02-06 23:53   ` David Matlack
2023-02-07 17:41     ` Vipin Sharma
2023-02-03 19:28 ` [Patch v2 3/5] KVM: x86/mmu: Optimize SPTE change for aging gfn range Vipin Sharma
2023-02-06 22:17   ` Ben Gardon
2023-02-07 17:48     ` Vipin Sharma
2023-02-06 23:56   ` David Matlack
2023-02-07 17:50     ` Vipin Sharma
2023-02-03 19:28 ` [Patch v2 4/5] KVM: x86/mmu: Remove handle_changed_spte_dirty_log() Vipin Sharma
2023-02-06 23:59   ` David Matlack
2023-02-07 17:51     ` Vipin Sharma
2023-02-03 19:28 ` [Patch v2 5/5] KVM: x86/mmu: Merge all handle_changed_pte* functions Vipin Sharma
2023-02-06 22:33   ` Ben Gardon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y+GKo0tI2NeKiNDR@google.com \
    --to=dmatlack@google.com \
    --cc=bgardon@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=vipinsh@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.