Re: [rfc patch 3/3] mm: munlock COW pages on truncation unmap

[KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>]

Hi

thanks for very interesting patches.
I have a question.


> @@ -835,6 +835,43 @@ static unsigned long zap_pte_range(struc
>  				    (page->index < details->first_index ||
>  				     page->index > details->last_index))
>  					continue;
> +				/*
> +				 * When truncating, private COW pages may be
> +				 * mlocked in VM_LOCKED VMAs, so they need
> +				 * munlocking here before getting freed.
> +				 *
> +				 * Skip them completely if we don't have the
> +				 * anon_vma locked.  We will get it the second
> +				 * time.  When page cache is truncated, no more
> +				 * private pages can show up against this VMA
> +				 * and the anon_vma is either present or will
> +				 * never be.
> +				 *
> +				 * Otherwise, we still have to synchronize
> +				 * against concurrent reclaimers.  We can not
> +				 * grab the page lock, but with correct
> +				 * ordering of page flag accesses we can get
> +				 * away without it.
> +				 *
> +				 * A concurrent isolator may add the page to
> +				 * the unevictable list, set PG_lru and then
> +				 * recheck PG_mlocked to verify it chose the
> +				 * right list and conditionally move it again.
> +				 *
> +				 * TestClearPageMlocked() provides one half of
> +				 * the barrier: when we do not see the page on
> +				 * the LRU and fail isolation, the isolator
> +				 * must see PG_mlocked cleared and move the
> +				 * page on its own back to the evictable list.
> +				 */
> +				if (private && !details->anon_vma)
> +					continue;
> +				if (private && TestClearPageMlocked(page)) {
> +					dec_zone_page_state(page, NR_MLOCK);
> +					count_vm_event(UNEVICTABLE_PGCLEARED);
> +					if (!isolate_lru_page(page))
> +						putback_lru_page(page);
> +				}
>  			}
>  			ptent = ptep_get_and_clear_full(mm, addr, pte,
>  							tlb->fullmm);

Umm..
I haven't understand this.

(1) unmap_mapping_range() is called twice.

	unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 1);
	truncate_inode_pages(mapping, new);
	unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 1);

(2) PG_mlock is turned on from mlock() and vmscan.
(3) vmscan grab anon_vma, but mlock don't grab anon_vma.
(4) after truncate_inode_pages(), we don't need to think vs-COW, because
    find_get_page() never success. but first unmap_mapping_range()
    have vs-COW racing. 

So, Is anon_vma grabbing really sufficient?
Or, you intent to the following?

	unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 0);
	truncate_inode_pages(mapping, new);
	unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 1);



> @@ -544,6 +544,13 @@ redo:
>  		 */
>  		lru = LRU_UNEVICTABLE;
>  		add_page_to_unevictable_list(page);
> +		/*
> +		 * See the TestClearPageMlocked() in zap_pte_range():
> +		 * if a racing unmapper did not see the above setting
> +		 * of PG_lru, we must see its clearing of PG_locked
> +		 * and move the page back to the evictable list.
> +		 */
> +		smp_mb();
>  	}

add_page_to_unevictable() have a spin lock. Why do we need additionl
explicit memory barrier?



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>