From: Andrew Jones <drjones@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Ben Gardon <bgardon@google.com>, kvm <kvm@vger.kernel.org>,
Paolo Bonzini <pbonzini@redhat.com>,
Christian Borntraeger <borntraeger@de.ibm.com>,
Janosch Frank <frankja@linux.ibm.com>
Subject: Re: [PATCH v2 02/11] KVM: selftests: Remove deadcode
Date: Fri, 13 Nov 2020 08:57:06 +0100 [thread overview]
Message-ID: <20201113075706.6lteawooppchxsda@kamzik.brq.redhat.com> (raw)
In-Reply-To: <20201112185006.GY26342@xz-x1>
On Thu, Nov 12, 2020 at 01:50:06PM -0500, Peter Xu wrote:
> On Thu, Nov 12, 2020 at 10:34:11AM -0800, Ben Gardon wrote:
> > I didn't review this patch closely enough, and assumed the clear dirty
> > log was still being done because of
> > afdb19600719 KVM: selftests: Use a single binary for dirty/clear log test
> >
> > Looking back now, I see that that is not the case.
> >
> > I'd like to retract my endorsement in that case. I'd prefer to leave
> > the dead code in and I'll send another series to actually use it once
> > this series is merged. I've already written the code to use it and
> > time the clearing, so it seems a pity to remove it now.
> >
> > Alternatively I could just revert this commit in that future series,
> > though I suspect not removing the dead code would reduce the chances
> > of merge conflicts. Either way works.
> >
> > I can extend the dirty log mode functions from dirty_log_test for
> > dirty_log_perf_test in that series too.
>
> Or... we can just remove all the "#ifdef" lines but assuming clear dirty log is
> always there? :) Assuming that is still acceptable as long as the test is
> matching latest kernel which definitely has the clear dirty log capability.
>
> It's kind of weird to test get-dirty-log perf without clear dirty log, since
> again if anyone really cares about the perf of that, then imho they should
> first switch to a new kernel with clear dirty log, rather than measuring the
> world without clear dirty log..
>
I have no opinion on this other than I'd prefer KVM selftests to not have
deadcode. If it makes more sense to fix the deadcode by bringing it back
to life than to drop the code, then please send patches to do that, at
which point I'd be happy to recommend dropping this patch.
Thanks,
drew
next prev parent reply other threads:[~2020-11-13 7:57 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-11-11 12:26 [PATCH v2 00/11] KVM: selftests: Cleanups, take 2 Andrew Jones
2020-11-11 12:26 ` [PATCH v2 01/11] KVM: selftests: Update .gitignore Andrew Jones
2020-11-12 18:12 ` Peter Xu
2020-11-13 7:52 ` Andrew Jones
2020-11-11 12:26 ` [PATCH v2 02/11] KVM: selftests: Remove deadcode Andrew Jones
2020-11-12 18:19 ` Peter Xu
2020-11-12 18:34 ` Ben Gardon
2020-11-12 18:50 ` Peter Xu
2020-11-13 7:57 ` Andrew Jones [this message]
2020-11-13 16:36 ` Paolo Bonzini
2020-11-11 12:26 ` [PATCH v2 03/11] KVM: selftests: Factor out guest mode code Andrew Jones
2020-11-11 22:35 ` Ben Gardon
2020-11-12 18:19 ` Peter Xu
2020-11-11 12:26 ` [PATCH v2 04/11] KVM: selftests: Make vm_create_default common Andrew Jones
2020-11-11 12:26 ` [PATCH v2 05/11] KVM: selftests: Introduce vm_create_[default_]_with_vcpus Andrew Jones
2020-11-11 12:26 ` [PATCH v2 06/11] KVM: selftests: dirty_log_test: Remove create_vm Andrew Jones
2020-11-11 22:46 ` Ben Gardon
2020-11-12 18:20 ` Peter Xu
2020-11-13 16:42 ` Paolo Bonzini
2020-11-16 12:16 ` Andrew Jones
2020-11-16 12:24 ` Paolo Bonzini
2020-11-11 12:26 ` [PATCH v2 07/11] KVM: selftests: Use vm_create_with_vcpus in create_vm Andrew Jones
2020-11-11 22:51 ` Ben Gardon
2020-11-11 12:26 ` [PATCH v2 08/11] KVM: selftests: Implement perf_test_util more conventionally Andrew Jones
2020-11-11 23:08 ` Ben Gardon
2020-11-12 9:06 ` Andrew Jones
2020-11-12 18:22 ` Peter Xu
2020-11-11 12:26 ` [PATCH v2 09/11] KVM: selftests: Also build dirty_log_perf_test on AArch64 Andrew Jones
2020-11-12 18:22 ` Peter Xu
2020-11-11 12:26 ` [PATCH v2 10/11] KVM: selftests: x86: Set supported CPUIDs on default VM Andrew Jones
2020-11-12 18:10 ` Peter Xu
2020-11-11 12:26 ` [PATCH v2 11/11] KVM: selftests: Make test skipping consistent Andrew Jones
2020-11-12 18:23 ` Peter Xu
2020-11-13 16:48 ` [PATCH v2 00/11] KVM: selftests: Cleanups, take 2 Paolo Bonzini
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=20201113075706.6lteawooppchxsda@kamzik.brq.redhat.com \
--to=drjones@redhat.com \
--cc=bgardon@google.com \
--cc=borntraeger@de.ibm.com \
--cc=frankja@linux.ibm.com \
--cc=kvm@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterx@redhat.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.