Re: [PATCH v2 2/2] KVM: selftests: access_tracking_perf_test: add option to skip the sanity check

[Sean Christopherson <seanjc@google.com>]

On Wed, Mar 26, 2025, James Houghton wrote:
> On Wed, Mar 26, 2025 at 11:41 AM Sean Christopherson <seanjc@google.com> wrote:
> > Then the auto resolving works as below, and as James pointed out, the assert
> > becomes
> >
> >                 TEST_ASSERT(!warn_only, ....);
> 
> I think the auto-resolving below needs to be flipped, and the
> TEST_ASSERT should be for `warn_only`, not `!warn_only`.
> 
> If warn_only == 1, the assert should pass.

/facepalm, yep

> > > > +                       break;
> > > >                 case 'h':
> > > >                 default:
> > > >                         help(argv[0]);
> > > > @@ -386,6 +394,23 @@ int main(int argc, char *argv[])
> > > >         page_idle_fd = open("/sys/kernel/mm/page_idle/bitmap", O_RDWR);
> > > >         __TEST_REQUIRE(page_idle_fd >= 0,
> > > >                        "CONFIG_IDLE_PAGE_TRACKING is not enabled");
> > > > +       if (warn_on_too_many_idle_pages == -1) {
> > > > +#ifdef __x86_64__
> > > > +               if (this_cpu_has(X86_FEATURE_HYPERVISOR)) {
> > > > +                       printf("Skipping idle page count sanity check, because the test is run nested\n");
> > > > +                       warn_on_too_many_idle_pages = 0;
> > > > +               } else
> > > > +#endif
> > > > +               if (is_numa_balancing_enabled()) {
> > > > +                       printf("Skipping idle page count sanity check, because NUMA balance is enabled\n");
> > > > +                       warn_on_too_many_idle_pages = 0;
> > > > +               } else {
> > > > +                       warn_on_too_many_idle_pages = 1;
> > > > +               }
> > > > +       } else if (!warn_on_too_many_idle_pages) {
> > > > +               printf("Skipping idle page count sanity check, because this was requested by the user\n");
> >
> > Eh, I vote to omit this.  The sanity check is still there, it's just degraded to
> > a warn.  I'm not totally against it, just seems superfluous and potentially confusing.
> 
> I agree, it's not adding much.
> 
> Separately: I've finished the MGLRU version of this test. It uses
> MGLRU if it is available, and marking pages as idle is much faster
> when using it. If MGLRU is enabled but otherwise not usable, the test
> fails, as the idle page bitmap is no longer usable for this test.
> 
> I'm happy to post a new version of Maxim's patch with the MGLRU
> patches too, Maxim, if you're okay with that.