Re: Excessive TLB flush ranges

[Thomas Gleixner <tglx@linutronix.de>]

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