Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753813AbaBYShj (ORCPT ); Tue, 25 Feb 2014 13:37:39 -0500 Received: from mail-ve0-f176.google.com ([209.85.128.176]:36099 "EHLO mail-ve0-f176.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753796AbaBYShi (ORCPT ); Tue, 25 Feb 2014 13:37:38 -0500 MIME-Version: 1.0 In-Reply-To: <1393352206.2577.36.camel@buesod1.americas.hpqcorp.net> References: <1393352206.2577.36.camel@buesod1.americas.hpqcorp.net> Date: Tue, 25 Feb 2014 10:37:37 -0800 X-Google-Sender-Auth: 1HY-HULnArUwCFERnIFOYD_ScIE Message-ID: Subject: Re: [PATCH v2] mm: per-thread vma caching From: Linus Torvalds To: Davidlohr Bueso Cc: Andrew Morton , Ingo Molnar , Peter Zijlstra , Michel Lespinasse , Mel Gorman , Rik van Riel , KOSAKI Motohiro , "Chandramouleeswaran, Aswin" , "Norton, Scott J" , linux-mm , Linux Kernel Mailing List Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Feb 25, 2014 at 10:16 AM, Davidlohr Bueso wrote: > index a17621c..14396bf 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -363,7 +363,12 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) > > mm->locked_vm = 0; > mm->mmap = NULL; > - mm->mmap_cache = NULL; > + mm->vmacache_seqnum = oldmm->vmacache_seqnum + 1; > + > + /* deal with overflows */ > + if (unlikely(mm->vmacache_seqnum == 0)) > + vmacache_invalidate_all(); Correct me if I'm wrong, but this can not possibly be correct. vmacache_invalidate_all() walks over all the threads of the current process, but "mm" here is the mm of the *new* process that is getting created, and is unrelated in all ways to the threads of the old process. So it walks completely the wrong list of threads. In fact, the sequence number of the old vm and the sequence number of the new vm cannot in any way be related. As far as I can tell, the only sane thing to do at fork/clone() time is to: - clear all the cache entries (of the new 'struct task_struct'! - so not in dup_mmap, but make sure it's zeroed when allocating!)( - set vmcache_seqnum to 0 in dup_mmap (since any sequence number is fine when it got invalidated, and 0 is best for "avoid overflow"). but I haven't thought deeply about this, but I pretty much guarantee that the quoted sequence above is wrong as-is. Linus -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/