linux-kernel - Re: [PATCH v3] mm: simplify find_vma

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Mon, 12 Dec 2011 14:34:01 -0500
From:	KOSAKI Motohiro <kosaki.motohiro@...il.com>
To:	Michal Hocko <mhocko@...e.cz>
CC:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Hugh Dickins <hughd@...gle.com>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Shaohua Li <shaohua.li@...el.com>
Subject: Re: [PATCH v3] mm: simplify find_vma_prev

(12/12/11 2:24 PM), KOSAKI Motohiro wrote:
>>> Why have you removed this guard? Previously we had pprev==NULL and
>>> returned mm->mmap.
>>> This seems like a semantic change without any explanation. Could you
>>> clarify?
>>
>> Scratch that. I have misread the code. find_vma will return mm->mmap if
>> the given address is bellow all vmas. Sorry about noise.
>>
>> The only concern left would be the caching. Are you sure this will not
>> break some workloads which benefit from mmap_cache usage and would
>> interfere with find_vma_prev callers now? Anyway this could be fixed
>> trivially.
>
> Here is callers list.
>
> find_vma_prev     115 arch/ia64/mm/fault.c      vma =
> find_vma_prev(mm, address,&prev_vma);
> find_vma_prev     183 arch/parisc/mm/fault.c    vma =
> find_vma_prev(mm, address,&prev_vma);
> find_vma_prev     229 arch/tile/mm/hugetlbpage.c                vma =
> find_vma_prev(mm, addr,&prev_vma);
> find_vma_prev     336 arch/x86/mm/hugetlbpage.c                 if
> (!(vma = find_vma_prev(mm, addr,&prev_vma)))
> find_vma_prev     388 mm/madvise.c      vma =
> find_vma_prev(current->mm, start,&prev);
> find_vma_prev     642 mm/mempolicy.c    vma = find_vma_prev(mm, start,&prev);
> find_vma_prev     388 mm/mlock.c        vma =
> find_vma_prev(current->mm, start,&prev);
> find_vma_prev     265 mm/mprotect.c     vma =
> find_vma_prev(current->mm, start,&prev);
>
> In short, find_find_prev() is only used from page fault, madvise, mbind, mlock
> and mprotect. And page fault is only performance impact callsite because other
> don't used frequently on regular workload.
>
> So, I wouldn't say, this patch has zero negative impact, but I think
> it is enough
> small and benefit is enough much.

In addition, other callsite (i.e. madvise, mbind, mlock and mprotect) are
used from syscall. then, an optimal behavior depend on syscall argument.
IOW, we and the kernel can't know it on ahead.

Therefore, this change may increase some applications performance a bit 
and may decrease another some applications. I can reasonably guess the
former are much than latter because many app have locality. but I can't
prove it.

Anyway, the impact is enough small, I think. They are rare than page fault.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/