From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757494AbbFQBYB (ORCPT ); Tue, 16 Jun 2015 21:24:01 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33875 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755649AbbFQBXx (ORCPT ); Tue, 16 Jun 2015 21:23:53 -0400 Date: Wed, 17 Jun 2015 03:22:45 +0200 From: Oleg Nesterov To: Al Viro Cc: Andrew Morton , Benjamin LaHaise , Jeff Moyer , linux-aio@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 0/3] aio: ctx->dead cleanups Message-ID: <20150617012245.GA22709@redhat.com> References: <20150616230414.GA15776@redhat.com> <20150617003906.GC17109@ZenIV.linux.org.uk> <20150617005012.GD17109@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150617005012.GD17109@ZenIV.linux.org.uk> User-Agent: Mutt/1.5.18 (2008-05-17) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 06/17, Al Viro wrote: > > On Wed, Jun 17, 2015 at 01:39:06AM +0100, Al Viro wrote: > > > Huh? kill_ioctx() picks ctx->mmap_base and passes it to vm_munmap(). > > Which tries to grab mmap_sem, blocks for mremap() from another thread > > and waits for it to drop mmap_sem. By that time ctx->mmap_base has > > nothing whatsoever to the argument we'd passed to vm_munmap(). Sure, > > it had been recalculated by aio_ring_remap(), but it's too late for > > us - we'd already fetched the old value. > > And yes, the leak you've spotted is real, but I would very much prefer > to avoid that goto - something like this instead: > > diff --git a/mm/mremap.c b/mm/mremap.c > index 034e2d3..b36b530 100644 > --- a/mm/mremap.c > +++ b/mm/mremap.c > @@ -291,7 +291,10 @@ static unsigned long move_vma(struct vm_area_struct *vma, > if (err < 0) { > move_page_tables(new_vma, new_addr, vma, old_addr, > moved_len, true); > - return err; > + vma = new_vma; > + old_len = new_len; > + old_addr = new_addr; > + new_addr = err; Personally, I'd really prefer to factor out at least this move_page_tables() with six args. Although I agree, "goto previous_if" doesn't look nice too, this needs cleanup. But this is minor. I am already sleeping, most probably I misread this code. But it seems that there is another bug with VM_ACCOUNT. I'll recheck tomorrow and write another email. Oleg.