From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934199AbcKNQsd (ORCPT ); Mon, 14 Nov 2016 11:48:33 -0500 Received: from gum.cmpxchg.org ([85.214.110.215]:47734 "EHLO gum.cmpxchg.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752253AbcKNQsa (ORCPT ); Mon, 14 Nov 2016 11:48:30 -0500 Date: Mon, 14 Nov 2016 11:48:22 -0500 From: Johannes Weiner To: "Kirill A. Shutemov" Cc: Jan Kara , Andrew Morton , Linus Torvalds , linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@fb.com Subject: Re: [PATCH 1/6] mm: khugepaged: fix radix tree node leak in shmem collapse error path Message-ID: <20161114164822.GB5141@cmpxchg.org> References: <20161107190741.3619-1-hannes@cmpxchg.org> <20161107190741.3619-2-hannes@cmpxchg.org> <20161108095352.GH32353@quack2.suse.cz> <20161108161245.GA4020@cmpxchg.org> <20161111105921.GC19382@node.shutemov.name> <20161111122224.GA5090@quack2.suse.cz> <20161111163753.GH19382@node.shutemov.name> <20161114080744.GA2524@quack2.suse.cz> <20161114142902.GA10455@node.shutemov.name> <20161114155250.GB3291@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161114155250.GB3291@cmpxchg.org> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 14, 2016 at 10:52:50AM -0500, Johannes Weiner wrote: > On Mon, Nov 14, 2016 at 05:29:02PM +0300, Kirill A. Shutemov wrote: > > @@ -1400,7 +1400,9 @@ static void collapse_shmem(struct mm_struct *mm, > > PAGE_SIZE, 0); > > > > spin_lock_irq(&mapping->tree_lock); > > - > > + slot = radix_tree_lookup_slot(&mapping->page_tree, index); > > + VM_BUG_ON_PAGE(page != radix_tree_deref_slot_protected(slot, > > + &mapping->tree_lock), page); > > VM_BUG_ON_PAGE(page_mapped(page), page); > > That looks good to me. The slot may get relocated, but the content > shouldn't change with the page locked. > > Are you going to send a full patch with changelog and sign-off? If so, > please add: > > Acked-by: Johannes Weiner Just to clarify, this is in addition to my radix_tree_iter_next() change. The iterator still needs to be reloaded because the number of valid slots that come after the current one can change as well.