On 10/5/20 7:11 AM, Jarkko Sakkinen wrote:
> + unsigned long count = 0;
...
> + xas_lock(&xas);
> + xas_for_each(&xas, page, idx_end) {
> + if (++count % XA_CHECK_SCHED)
> + continue;
Let's slow down and think through the loop, please.
First time through the loop, count=0, XA_CHECK_SCHED=4096, it will do a
++count, and now count=1. (count % XA_CHECK_SCHED) == 1. It will
continue. It skips the page->vm_max_prot_bits checks.
Next time through the loop, count=1, XA_CHECK_SCHED=4096, it will do a
++count, and now count=2. (count % XA_CHECK_SCHED) == 2. It will
continue. It skips the page->vm_max_prot_bits checks.
...
It will do this until it hits count=4095 where it will actually fall
into the rest of the loop, doing the page->vm_max_prot_bits checks.
So, in the end the loop only does what it's supposed to be doing 1/4096
times. Not great. Don't we have tests that will notice breakage like this?
The XA_CHECK_SCHED needs to be stuck near the *end* of the loop, just
before the lock dropping and resched stuff.
