Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210830044425.2686755-1-mizhang@google.com> <20210830044425.2686755-3-mizhang@google.com>
 <CANgfPd_46=V24r5Qu8cDuOCwVRSEF9RFHuD-1sPpKrBCjWOA2w@mail.gmail.com>
 <YS5fxJtX/nYb43ir@google.com> <CAL715WJUmRJmt=u4Gi3ydTpbTGy2M5Wi=CbF9Qs8GNRK8g5FAA@mail.gmail.com>
In-Reply-To: <CAL715WJUmRJmt=u4Gi3ydTpbTGy2M5Wi=CbF9Qs8GNRK8g5FAA@mail.gmail.com>
From:   Mingwei Zhang <mizhang@google.com>
Date:   Wed, 8 Sep 2021 09:07:09 -0700
Message-ID: <CAL715WJYqusqiztS=fYE46jXmHUi9uni_kswt=ALyUx_hxKBFg@mail.gmail.com>
Subject: Re: [PATCH v3 2/2] selftests: KVM: use dirty logging to check if page
 stats work correctly
To:     Sean Christopherson <seanjc@google.com>
Cc:     Ben Gardon <bgardon@google.com>,
        Paolo Bonzini <pbonzini@redhat.com>, kvm <kvm@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        David Matlack <dmatlack@google.com>,
        Jing Zhang <jingzhangos@google.com>,
        Peter Xu <peterx@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Mon, Sep 6, 2021 at 1:05 PM Mingwei Zhang <mizhang@google.com> wrote:
>
> On Tue, Aug 31, 2021 at 9:58 AM Sean Christopherson <seanjc@google.com> wrote:
> >
> > On Mon, Aug 30, 2021, Ben Gardon wrote:
> > > On Sun, Aug 29, 2021 at 9:44 PM Mingwei Zhang <mizhang@google.com> wrote:
> > > > diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> > > > index af1031fed97f..07eb6b5c125e 100644
> > > > --- a/tools/testing/selftests/kvm/lib/test_util.c
> > > > +++ b/tools/testing/selftests/kvm/lib/test_util.c
> > > > @@ -15,6 +15,13 @@
> > > >  #include "linux/kernel.h"
> > > >
> > > >  #include "test_util.h"
> > > > +#include "processor.h"
> > > > +
> > > > +static const char * const pagestat_filepaths[] = {
> > > > +       "/sys/kernel/debug/kvm/pages_4k",
> > > > +       "/sys/kernel/debug/kvm/pages_2m",
> > > > +       "/sys/kernel/debug/kvm/pages_1g",
> > > > +};
> > >
> > > I think these should only be defined for x86_64 too. Is this the right
> > > file for these definitions or is there an arch specific file they
> > > should go in?
> >
> > The stats also need to be pulled from the selftest's VM, not from the overall KVM
> > stats, otherwise the test will fail if there are any other active VMs on the host,
> > e.g. I like to run to selftests and kvm-unit-tests in parallel.
>
> That is correct. But since this selftest is not the 'default' selftest
> that people normally run, can we make an assumption on running these
> tests at this moment? I am planning to submit this test and improve it
> in the next series by using Jing's fd based KVM stats interface to
> eliminate the assumption of the existence of a single running VM.
> Right now, this interface still needs some work, so I am taking a
> shortcut that directly uses the whole-system metricfs based interface.
>
> But I can choose to do that and submit the fd-based API together with
> this series. What do you suggest?


I will take my point back, since some of the "TEST_ASSERT" in this
selftest does assume that there is no other VM running even on
'default' case (ie., run ./dirty_logging_perf_test without arguments).
Therefore, this patch will make the test flaky.

I will go back to implement the fd based API and submit the code along
with this selftest.

Thanks.
-Mingwei