Re: [PATCH 3/6] mm: Defer TLB flush after unmap as long as possible

[Rik van Riel <riel@redhat.com>]

On 04/21/2015 06:41 AM, Mel Gorman wrote:
> If a PTE is unmapped and it's dirty then it was writable recently. Due
> to deferred TLB flushing, it's best to assume a writable TLB cache entry
> exists. With that assumption, the TLB must be flushed before any IO can
> start or the page is freed to avoid lost writes or data corruption. Prior
> to this patch, such PFNs were simply flushed immediately. In this patch,
> the caller is informed that such entries potentially exist and it's up to
> the caller to flush before pages are freed or IO can start.
> 
> Signed-off-by: Mel Gorman <mgorman@suse.de>

> @@ -1450,10 +1455,11 @@ static int page_not_mapped(struct page *page)
>   * page, used in the pageout path.  Caller must hold the page lock.
>   * Return values are:
>   *
> - * SWAP_SUCCESS	- we succeeded in removing all mappings
> - * SWAP_AGAIN	- we missed a mapping, try again later
> - * SWAP_FAIL	- the page is unswappable
> - * SWAP_MLOCK	- page is mlocked.
> + * SWAP_SUCCESS	       - we succeeded in removing all mappings
> + * SWAP_SUCCESS_CACHED - Like SWAP_SUCCESS but a writable TLB entry may exist
> + * SWAP_AGAIN	       - we missed a mapping, try again later
> + * SWAP_FAIL	       - the page is unswappable
> + * SWAP_MLOCK	       - page is mlocked.
>   */
>  int try_to_unmap(struct page *page, enum ttu_flags flags)
>  {
> @@ -1481,7 +1487,8 @@ int try_to_unmap(struct page *page, enum ttu_flags flags)
>  	ret = rmap_walk(page, &rwc);
>  
>  	if (ret != SWAP_MLOCK && !page_mapped(page))
> -		ret = SWAP_SUCCESS;
> +		ret = (ret == SWAP_AGAIN_CACHED) ? SWAP_SUCCESS_CACHED : SWAP_SUCCESS;
> +
>  	return ret;
>  }

This wants a big fat comment explaining why SWAP_AGAIN_CACHED is turned
into SWAP_SUCCESS_CACHED.

I think I understand why this is happening, but I am not sure how to
explain it...

> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 12ec298087b6..0ad3f435afdd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -860,6 +860,7 @@ static unsigned long shrink_page_list(struct list_head *page_list,
>  	unsigned long nr_reclaimed = 0;
>  	unsigned long nr_writeback = 0;
>  	unsigned long nr_immediate = 0;
> +	bool tlb_flush_required = false;
>  
>  	cond_resched();
>  
> @@ -1032,6 +1033,9 @@ static unsigned long shrink_page_list(struct list_head *page_list,
>  				goto keep_locked;
>  			case SWAP_MLOCK:
>  				goto cull_mlocked;
> +			case SWAP_SUCCESS_CACHED:
> +				/* Must flush before free, fall through */
> +				tlb_flush_required = true;
>  			case SWAP_SUCCESS:
>  				; /* try to free the page below */
>  			}
> @@ -1176,7 +1180,8 @@ keep:
>  	}
>  
>  	mem_cgroup_uncharge_list(&free_pages);
> -	try_to_unmap_flush();
> +	if (tlb_flush_required)
> +		try_to_unmap_flush();
>  	free_hot_cold_page_list(&free_pages, true);

Don't we have to flush the TLB before calling pageout() on the page?

In other words, we would also have to batch up calls to pageout(), if
we want to do batched TLB flushing.

This could be accomplished by putting the SWAP_SUCCESS_CACHED pages on
a special list, instead of calling pageout() on them immediately, and
then calling pageout() on all the pages on that list after the batch
flush.

-- 
All rights reversed