From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755114AbcKNTk7 (ORCPT ); Mon, 14 Nov 2016 14:40:59 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34903 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751532AbcKNTk6 (ORCPT ); Mon, 14 Nov 2016 14:40:58 -0500 Date: Mon, 14 Nov 2016 22:40:54 +0300 From: "Kirill A. Shutemov" To: Johannes Weiner 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: <20161114194054.GA12829@node.shutemov.name> References: <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> <20161114164822.GB5141@cmpxchg.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20161114164822.GB5141@cmpxchg.org> User-Agent: Mutt/1.5.23.1 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 14, 2016 at 11:48:22AM -0500, Johannes Weiner wrote: > 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. Could you just amend all these fixups into your patch? -- Kirill A. Shutemov