From: "Radim Krčmář" <rkrcmar@redhat.com>
To: "Longpeng (Mike)" <longpeng2@huawei.com>
Cc: pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com,
hpa@zytor.com, x86@kernel.org, kvm@vger.kernel.org,
linux-kernel@vger.kernel.org, zhaoshenglong@huawei.com,
richard.weiyang@huawei.com
Subject: Re: [PATCH] kvm: x86: correct the misleading comment in vmx_handle_external_intr
Date: Wed, 12 Oct 2016 16:05:09 +0200 [thread overview]
Message-ID: <20161012140508.GC30525@potion> (raw)
In-Reply-To: <57FD8EB9.9020400@huawei.com>
2016-10-12 09:15+0800, Longpeng (Mike):
> On 2016/10/12 2:23, Radim Krčmář wrote:
>> This part is acceptable as it gives a new information code, yet the
>> function does not modify flags, which makes it unremarkable.
>> And dependencies on the caller would be better described in a header
>> (if we cannot express them well in the code).
>>
>> The most comment-worthy thing about this function is the reason why we
>> execute the interrupt handler manually, i.e. the dependency on
>> VM_EXIT_ACK_INTR_ON_EXIT, but that is easy to tell from the commit
>> message and convenient access to git history is essential in a workflow,
>> so providing a leeway could be counter-productive.
>>
>> I would go with no comment for now.
>
> Thanks for your patience, and your advice is useful for me.
I appreciate the patch, I just didn't want to repeat the same mistake
that you were fixing in the patch, which made me go into rambling mode.
Please send v2 with a simpler code comment (or no comment).
And you are more than welcome to improve the code even further!
> In addition, the comment below is misleading too, hope you can fix it
> simultaneously.
>
> /* Interrupt is enabled by handle_external_intr() */
> kvm_x86_ops->handle_external_intr(vcpu);
Yep, this comment should have been expressed in a function name.
Paolo already fixed it in 1a6982353db9 ("KVM: x86: remove stale
comments").
prev parent reply other threads:[~2016-10-12 14:05 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-10-10 0:23 [PATCH] kvm: x86: correct the misleading comment in vmx_handle_external_intr Longpeng(Mike)
2016-10-11 18:23 ` Radim Krčmář
2016-10-12 1:15 ` Longpeng (Mike)
2016-10-12 14:05 ` Radim Krčmář [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=20161012140508.GC30525@potion \
--to=rkrcmar@redhat.com \
--cc=hpa@zytor.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longpeng2@huawei.com \
--cc=mingo@redhat.com \
--cc=pbonzini@redhat.com \
--cc=richard.weiyang@huawei.com \
--cc=tglx@linutronix.de \
--cc=x86@kernel.org \
--cc=zhaoshenglong@huawei.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.