kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Like Xu <like.xu.linux@gmail.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	Ingo Molnar <mingo@redhat.com>,
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 0/5] KVM: x86: Clean up redundant macro definitions
Date: Mon, 9 Aug 2021 15:08:35 +0000	[thread overview]
Message-ID: <YRFE8y24+5P31RjK@google.com> (raw)
In-Reply-To: <20210809093410.59304-1-likexu@tencent.com>

On Mon, Aug 09, 2021, Like Xu wrote:
> In KVM/x86 code, there are macros with the same name that are
> defined and used separately in the evolving code, and if the scope
> of the code review is only on iterations of the patch set, it can be
> difficult to spot these fragmented macros being defined repeatedly.
> 
> IMO, it's necessary to clean this up to improve the consistency and

Please use more specific shortlogs.  "Clean up" is too ambiguous, e.g. I would
expect it to mean "clean up the macro itself".

> readability of the code, and it also helps to avoid software defects
> caused by inconsistencies in the scope of influence of macros.
> 
> Like Xu (5):

>   KVM: x86: Clean up redundant mod_64(x, y) macro definition

Feels like we should find a home outside of KVM for mod_64(), it's a full generic
and straightforward macro.

>   KVM: x86: Clean up redundant CC macro definition

CC() should be kept where it is.  It should never be used outside of nested code
and is far too generic of a name to be exposed to the world at large.  It was
deliberately defined only in the nested.c files.

>   KVM: x86: Clean up redundant ROL16(val, n) macro definition

Moving ROL16 seems ok, though it scares me a bit that someone went through the
trouble of #undef'ing the macros.

>   KVM: x86: Clean up redundant __ex(x) macro definition

__ex() and __kvm_handle_fault_on_reboot() were supposed to have been removed.
The last two patches [8][9] of this series[*] got lost; I'll repost them as they
no longer apply cleanly.

>   KVM: x86: Clean up redundant pr_fmt(fmt) macro definition for svm

NAK, this breaks sev.c's formatting.  Arguably, nested.c and avic.c should use
more specific print messages, though that gets tricky with nested code as it's
not wholly contained in nested.c.

      parent reply	other threads:[~2021-08-09 15:08 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09  9:34 [PATCH 0/5] KVM: x86: Clean up redundant macro definitions Like Xu
2021-08-09  9:34 ` [PATCH 1/5] KVM: x86: Clean up redundant mod_64(x, y) macro definition Like Xu
2021-08-09  9:34 ` [PATCH 2/5] KVM: x86: Clean up redundant CC " Like Xu
2021-08-10 17:47   ` Paolo Bonzini
2021-08-09  9:34 ` [PATCH 3/5] KVM: x86: Clean up redundant ROL16(val, n) " Like Xu
2021-08-10 17:49   ` Paolo Bonzini
2021-08-09  9:34 ` [PATCH 4/5] KVM: x86: Clean up redundant __ex(x) " Like Xu
2021-08-09  9:34 ` [PATCH 5/5] KVM: x86: Clean up redundant pr_fmt(fmt) macro definition for svm Like Xu
2021-08-10 17:48   ` Paolo Bonzini
2021-08-12 13:00     ` Like Xu
2021-08-09 15:08 ` Sean Christopherson [this message]

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=YRFE8y24+5P31RjK@google.com \
    --to=seanjc@google.com \
    --cc=jmattson@google.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=like.xu.linux@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=vkuznets@redhat.com \
    --cc=wanpengli@tencent.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).