All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gleb Natapov <gleb@redhat.com>
To: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
Cc: mtosatti@redhat.com, kvm@vger.kernel.org
Subject: Re: [PATCH 1/8] KVM: MMU: Fix and clean up for_each_gfn_* macros
Date: Mon, 28 Jan 2013 14:24:25 +0200	[thread overview]
Message-ID: <20130128122425.GC22871@redhat.com> (raw)
In-Reply-To: <20130123191321.e280cec8.yoshikawa_takuya_b1@lab.ntt.co.jp>

On Wed, Jan 23, 2013 at 07:13:21PM +0900, Takuya Yoshikawa wrote:
> The expression (sp)->gfn should not be expanded using @gfn.
> 
> Although no user of these macros passes a string other than gfn now,
> this should be fixed before anyone sees strange errors.
> 
> Also, the counter-intuitive conditions, which had been used before these
> macros were introduced to avoid extra indentations, should not be used.
> 
Not sure what do you mean here. This counter-intuitive conditions are
used so that if "else" follows the macro it will not be interpreted as
belonging to the hidden if(). Like in the following code:

 if (x)
    for_each_gfn_sp()
 else
   do_y();

You do not expect do_y() to be called for each sp->gfn != gfn.

> Note: ignored the following checkpatch report:
>   ERROR: Macros with complex values should be enclosed in parenthesis
> 
> Signed-off-by: Takuya Yoshikawa <yoshikawa_takuya_b1@lab.ntt.co.jp>
> ---
>  arch/x86/kvm/mmu.c |   18 ++++++++----------
>  1 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c
> index 9f628f7..376cec8 100644
> --- a/arch/x86/kvm/mmu.c
> +++ b/arch/x86/kvm/mmu.c
> @@ -1658,16 +1658,14 @@ static int kvm_mmu_prepare_zap_page(struct kvm *kvm, struct kvm_mmu_page *sp,
>  static void kvm_mmu_commit_zap_page(struct kvm *kvm,
>  				    struct list_head *invalid_list);
>  
> -#define for_each_gfn_sp(kvm, sp, gfn, pos)				\
> -  hlist_for_each_entry(sp, pos,						\
> -   &(kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)], hash_link)	\
> -	if ((sp)->gfn != (gfn)) {} else
> -
> -#define for_each_gfn_indirect_valid_sp(kvm, sp, gfn, pos)		\
> -  hlist_for_each_entry(sp, pos,						\
> -   &(kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(gfn)], hash_link)	\
> -		if ((sp)->gfn != (gfn) || (sp)->role.direct ||		\
> -			(sp)->role.invalid) {} else
> +#define for_each_gfn_sp(_kvm, _sp, _gfn, _pos)				\
> +	hlist_for_each_entry(_sp, _pos,					\
> +	  &(_kvm)->arch.mmu_page_hash[kvm_page_table_hashfn(_gfn)], hash_link) \
> +		if ((_sp)->gfn == (_gfn))
> +
> +#define for_each_gfn_indirect_valid_sp(_kvm, _sp, _gfn, _pos)		\
> +	for_each_gfn_sp(_kvm, _sp, _gfn, _pos)				\
> +		if (!(_sp)->role.direct && !(_sp)->role.invalid)
>  
>  /* @sp->gfn should be write-protected at the call site */
>  static int __kvm_sync_page(struct kvm_vcpu *vcpu, struct kvm_mmu_page *sp,
> -- 
> 1.7.5.4

--
			Gleb.

  reply	other threads:[~2013-01-28 12:24 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-23 10:12 [PATCH 0/8] KVM: Reduce mmu_lock hold time when zapping mmu pages Takuya Yoshikawa
2013-01-23 10:13 ` [PATCH 1/8] KVM: MMU: Fix and clean up for_each_gfn_* macros Takuya Yoshikawa
2013-01-28 12:24   ` Gleb Natapov [this message]
2013-01-28 12:29     ` Takuya Yoshikawa
2013-01-23 10:13 ` [PATCH 2/8] KVM: MMU: Use list_for_each_entry_safe in kvm_mmu_commit_zap_page() Takuya Yoshikawa
2013-01-23 10:14 ` [PATCH 3/8] KVM: MMU: Add a parameter to kvm_mmu_prepare_zap_page() to update the next position Takuya Yoshikawa
2013-01-23 10:15 ` [PATCH 4/8] KVM: MMU: Introduce for_each_gfn_indirect_valid_sp_safe macro Takuya Yoshikawa
2013-01-23 10:16 ` [PATCH 5/8] KVM: MMU: Delete hash_link node in kvm_mmu_prepare_zap_page() Takuya Yoshikawa
2013-01-23 10:16 ` [PATCH 6/8] KVM: MMU: Introduce free_zapped_mmu_pages() for freeing mmu pages in a list Takuya Yoshikawa
2013-01-23 10:17 ` [PATCH 7/8] KVM: MMU: Split out free_zapped_mmu_pages() from kvm_mmu_commit_zap_page() Takuya Yoshikawa
2013-01-23 10:18 ` [PATCH 8/8] KVM: MMU: Move free_zapped_mmu_pages() out of the protection of mmu_lock Takuya Yoshikawa
2013-02-04 13:50   ` Marcelo Tosatti
2013-02-05  2:21     ` Takuya Yoshikawa
2013-01-23 10:44 ` [PATCH 0/8] KVM: Reduce mmu_lock hold time when zapping mmu pages Xiao Guangrong
2013-01-23 13:28   ` Takuya Yoshikawa
2013-01-23 13:45     ` Xiao Guangrong
2013-01-23 14:49       ` Takuya Yoshikawa
2013-01-23 15:45         ` Xiao Guangrong
2013-02-04 13:42   ` Marcelo Tosatti
2013-02-05  5:30     ` Xiao Guangrong
2013-02-04 13:29 ` Marcelo Tosatti

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=20130128122425.GC22871@redhat.com \
    --to=gleb@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=mtosatti@redhat.com \
    --cc=yoshikawa_takuya_b1@lab.ntt.co.jp \
    /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.