All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ben Gardon <bgardon@google.com>
To: Peter Xu <peterx@redhat.com>
Cc: Andrew Jones <drjones@redhat.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: Thu, 12 Nov 2020 10:34:11 -0800	[thread overview]
Message-ID: <CANgfPd_R_Rjn+QT_yiUwpCUK3TUfmhSN6XpZ5=L17mhrtMi7Zw@mail.gmail.com> (raw)
In-Reply-To: <20201112181921.GS26342@xz-x1>

On Thu, Nov 12, 2020 at 10:19 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Wed, Nov 11, 2020 at 01:26:27PM +0100, Andrew Jones wrote:
> > Nothing sets USE_CLEAR_DIRTY_LOG anymore, so anything it surrounds
> > is dead code.
> >
> > Reviewed-by: Ben Gardon <bgardon@google.com>
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
>
> It's kind of a pity that there seem to a few valid measurements for clear dirty
> log from Ben. I'm just thinking whether clear dirty log should be even more
> important since imho that should be the right way to use KVM_GET_DIRTY_LOG on a
> kernel new enough, since it's a total win (not like dirty ring, which depends).

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.


>
> So far, the statement is definitely true above, since we can always work on
> top.  So:
>
> Reviewed-by: Peter Xu <peterx@redhat.com>
>
> Thanks,
>
> --
> Peter Xu
>

  reply	other threads:[~2020-11-12 18:34 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 [this message]
2020-11-12 18:50       ` Peter Xu
2020-11-13  7:57         ` Andrew Jones
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='CANgfPd_R_Rjn+QT_yiUwpCUK3TUfmhSN6XpZ5=L17mhrtMi7Zw@mail.gmail.com' \
    --to=bgardon@google.com \
    --cc=borntraeger@de.ibm.com \
    --cc=drjones@redhat.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.