On Wed, Mar 26, 2025, James Houghton wrote:
> On Wed, Mar 26, 2025 at 11:41 AM Sean Christopherson <sea...@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.
