Re: [PATCH] iommupt/kunit: Read an exact NR_SECONDARY_PAGETABLE snapshot for the leak check

[fangyu.yu@linux.alibaba.com]

>> >> +/*
>> >> + * The global NR_SECONDARY_PAGETABLE counter is batched per-CPU and folded
>> >> + * into the global value asynchronously, sum the global value and the per-CPU
>> >> + * deltas to get an exact snapshot.
>> >> + *
>> >> + * The global value must be read without the clamp of global_node_page_state():
>> >> + * the raw counter is transiently negative whenever a folded free batch
>> >> + * outweighs allocation deltas still pending on other CPUs, and clamping
>> >> + * before adding the deltas back would overestimate and report phantom leaks.
>> >> + * global_node_page_state_monotonic() is normally reserved for monotonic
>> >> + * counters because a raw negative read is garbage as a page count, but the
>> >> + * sum with the pending deltas always restores the exact non-negative value.
>> >> + */
>> >> +static unsigned long pt_kunit_nr_secondary_pagetable(void)
>> >> +{
>> >> +	unsigned long nr = global_node_page_state_monotonic(NR_SECONDARY_PAGETABLE);
>> >> +
>> >> +#ifdef CONFIG_SMP
>> >> +	int nid, cpu;
>> >> +
>> >> +	cpus_read_lock();
>> >> +	for_each_online_cpu(cpu)
>> >> +		for_each_online_node(nid)
>> >> +			nr += per_cpu_ptr(NODE_DATA(nid)->per_cpu_nodestats,
>> >> +					  cpu)->vm_node_stat_diff[NR_SECONDARY_PAGETABLE];
>> >> +	cpus_read_unlock();
>> >> +#endif
>> >
>> >This should not be outside the mm code that manages this data
>> >structure.
>> >
>>
>> You're right. However, I couldn't find a way to solve this by
>> modifying only the iommupt KUnit code.
>>
>> >I think the kunit runner executes this in some environment that
>> >doesn't trigger this delay? At least I've never seen this assert?
>> >
>>
>> I can reproduce the assert stably on an x86 machine by booting a
>> 4-core riscv64 QEMU.
>
>Possibly kunit runner doesn't enable smp. Can you investigate? Maybe
>we can solve this by disabling the check on !SMP
>

Hi Jason,

I looked into this, and you're right: the default KUnit config does
not enable SMP. All runs below were done on the current tree without
the proposed fix.

1) Default KUnit command, without SMP - the tests pass:

  $ ./tools/testing/kunit/kunit.py run 'riscv_64_iommu*' \
        --arch x86_64 --kunitconfig drivers/iommu/generic_pt

The generated .config contains "# CONFIG_SMP is not set", and the
run completes with:

  Testing complete. Ran 21 tests: passed: 18, skipped: 3

2) The same command with SMP enabled - the tests fail:

  $ ./tools/testing/kunit/kunit.py run 'riscv_64_iommu*' \
        --arch x86_64 --kunitconfig drivers/iommu/generic_pt \
        --kconfig_add CONFIG_SMP=y --qemu_args="-smp 4"

The run fails consistently. In the example above, 14 of 21 tests
failed. With the proposed fix, the same command passes with 18
passed and 3 skipped.

3) Could you clarify what you meant by "disabling the check on !SMP"?

Thanks,
Fangyu

>Jason