On Tue, May 16 2023 at 16:21, Thomas Gleixner wrote:
> On Tue, May 16 2023 at 18:05, Baoquan He wrote:
>> On 05/16/23 at 11:03am, Thomas Gleixner wrote:
>>> static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end)
>>> {
>>> - unsigned long resched_threshold;
>>> + unsigned long resched_threshold, num_entries = 0, num_alias_entries = 0;
>>> + struct vmap_area alias_va = { .va_start = start, .va_end = end };
>>
>> Note that the start and end passed in are not only direct map which is
>> alias of va. It is the range which has done merging on direct map range
>> and all dirty range of vbq in _vm_unmap_aliases(). We may need to append
>> below draft code on your patch to at least flush the direct map range
>> separately.
>
> Indeed. Missed that one. What a maze.
Staring more at this vbq handling in _vm_unmap_aliases():
It iterates over all possible CPUs and accounts a range when
if (vb->dirty && vb->dirty != VMAP_BBMAP_BITS)
so that it gets flushed, but it does not modify that data at all, so a
concurrent invocation on a different CPU will see the same ranges and
flush them too, right?
Now after this loop _vm_unmap_aliases() invokes
purge_fragmented_blocks_allcpus(), but this time under vmap_purge_lock.
purge_fragmented_blocks_allcpus() iterates over all possible CPUs once
again iterate the same per CPU queues in order to potentially free
blocks.
I'm probably missing something, but can't this be _one_ loop which
handles all of this under vmap_purge_lock?
Aside of that, if I read the code correctly then if there is an unmap
via vb_free() which does not cover the whole vmap block then vb->dirty
is set and every _vm_unmap_aliases() invocation flushes that dirty range
over and over until that vmap block is completely freed, no?
Thanks,
tglx
