Excessive TLB flush ranges

[Thomas Gleixner]

On Wed, May 17 2023 at 18:52, Baoquan He wrote:
> On 05/17/23 at 11:38am, Thomas Gleixner wrote:
>> On Tue, May 16 2023 at 21:03, Thomas Gleixner wrote:
>> >
>> > 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?
>> 
>> Something like the below would cure that.
>> 
>> While it prevents that this is flushed forever it does not cure the
>> eventually overly broad flush when the block is completely dirty and
>> purged:
>> 
>> Assume a block with 1024 pages, where 1022 pages are already freed and
>> TLB flushed. Now the last 2 pages are freed and the block is purged,
>> which results in a flush of 1024 pages where 1022 are already done,
>> right?
>
> This is good idea, I am thinking how to reply to your last mail and how
> to fix this. While your cure code may not work well. Please see below
> inline comment.

See below.

> One vmap block has 64 pages.
> #define VMAP_MAX_ALLOC          BITS_PER_LONG   /* 256K with 4K pages */

No, VMAP_MAX_ALLOC is the allocation limit for a single vb_alloc().

On 64bit it has at least 128 pages, but can have up to 1024:

#define VMAP_BBMAP_BITS_MAX	1024	/* 4MB with 4K pages */
#define VMAP_BBMAP_BITS_MIN	(VMAP_MAX_ALLOC*2)

and then some magic happens to calculate the actual size

#define VMAP_BBMAP_BITS		\
		VMAP_MIN(VMAP_BBMAP_BITS_MAX,	\
		VMAP_MAX(VMAP_BBMAP_BITS_MIN,	\
			VMALLOC_PAGES / roundup_pow_of_two(NR_CPUS) / 16))

which is in a range of (2*BITS_PER_LONG) ... 1024.

The actual vmap block size is:

#define VMAP_BLOCK_SIZE		(VMAP_BBMAP_BITS * PAGE_SIZE)

Which is then obviously something between 512k and 4MB on 64bit and
between 256k and 4MB on 32bit.

>> @@ -2240,13 +2240,17 @@ static void _vm_unmap_aliases(unsigned l
>>  		rcu_read_lock();
>>  		list_for_each_entry_rcu(vb, &vbq->free, free_list) {
>>  			spin_lock(&vb->lock);
>> -			if (vb->dirty && vb->dirty != VMAP_BBMAP_BITS) {
>> +			if (vb->dirty_max && vb->dirty != VMAP_BBMAP_BITS) {
>>  				unsigned long va_start = vb->va->va_start;
>>  				unsigned long s, e;
>
> When vb_free() is invoked, it could cause three kinds of vmap_block as
> below. Your code works well for the 2nd case, for the 1st one, it may be
> not. And the 2nd one is the stuff that we reclaim and put into purge
> list in purge_fragmented_blocks_allcpus().
>
> 1)
>   |-----|------------|-----------|-------|
>   |dirty|still mapped|   dirty   | free  |
>
> 2)
>   |------------------------------|-------|
>   |         dirty                | free  |


You sure? The first one is put into the purge list too.

   /* Expand dirty range */
   vb->dirty_min = min(vb->dirty_min, offset);
   vb->dirty_max = max(vb->dirty_max, offset + (1UL << order));

                 pages bits   dirtymin            dirtymax
   vb_alloc(A)   2     0 - 1  VMAP_BBMAP_BITS     0
   vb_alloc(B)   4     2 - 5
   vb_alloc(C)   2     6 - 7

So you get three variants:

1) Flush after freeing A

   vb_free(A)    2     0 - 1  0			  1
   Flush                      VMAP_BBMAP_BITS     0    <- correct
   vb_free(C)    2     6 - 7  6                   7
   Flush                      VMAP_BBMAP_BITS     0    <- correct


2) No flush between freeing A and C

   vb_free(A)    2     0 - 1  0			  1
   vb_free(C)    2     6 - 7  0                   7     
   Flush                      VMAP_BBMAP_BITS     0     <- overbroad flush
   

3) No flush between freeing A, C, B

   vb_free(A)    2     0 - 1  0			  1
   vb_free(C)    2     6 - 7  0                   7
   vb_free(C)    2     2 - 5  0                   7
   Flush                      VMAP_BBMAP_BITS     0    <- correct

So my quick hack makes it correct for #1 and #3 and prevents repeated
flushes of already flushed areas.

To prevent #2 you need a bitmap which keeps track of the flushed areas.

Thanks,

        tglx