Re: [PATCH] THP: mremap support and TLB optimization #2

[Johannes Weiner <jweiner@redhat.com>]

On Thu, Jul 28, 2011 at 04:26:31PM +0200, Andrea Arcangeli wrote:
> Hello,
> 
> this is the latest version of the mremap THP native implementation
> plus optimizations.
> 
> So first question is: am I right, the "- 1" that I am removing below
> was buggy? It's harmless because these old_end/next are page aligned,
> but if PAGE_SIZE would be 1, it'd break, right? It's really confusing
> to read even if it happens to work. Please let me know because that "-
> 1" ironically it's the thing I'm less comfortable about in this patch.

> @@ -134,14 +126,17 @@ unsigned long move_page_tables(struct vm
>  {
>  	unsigned long extent, next, old_end;
>  	pmd_t *old_pmd, *new_pmd;
> +	bool need_flush = false;
>  
>  	old_end = old_addr + len;
>  	flush_cache_range(vma, old_addr, old_end);
>  
> +	mmu_notifier_invalidate_range_start(vma->vm_mm, old_addr, old_end);
> +
>  	for (; old_addr < old_end; old_addr += extent, new_addr += extent) {
>  		cond_resched();
>  		next = (old_addr + PMD_SIZE) & PMD_MASK;
> -		if (next - 1 > old_end)
> +		if (next > old_end)
>  			next = old_end;

If old_addr + PMD_SIZE overflows, next will be zero, thus smaller than
old_end and not fixed up.  This results in a bogus extent length here:

>  		extent = next - old_addr;

which I think can overrun old_addr + len if the extent should have
actually been smaller than the distance between new_addr and the next
PMD as well as that LATENCY_LIMIT to which extent is capped a few
lines below.  I haven't checked all the possibilities, though.

It could probably be

	if (next > old_end || next - 1 > old_end)
		next = old_end

to catch the theoretical next == old_end + 1 case, but PAGE_SIZE > 1
looks like a sensible assumption to me.

--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>