From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id CEF11E77188 for ; Wed, 18 Dec 2024 10:06:13 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5DFE96B0082; Wed, 18 Dec 2024 05:06:13 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 58F896B0083; Wed, 18 Dec 2024 05:06:13 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 47E3A6B0085; Wed, 18 Dec 2024 05:06:13 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 2BE9A6B0082 for ; Wed, 18 Dec 2024 05:06:13 -0500 (EST) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id A9D2CA0D50 for ; Wed, 18 Dec 2024 10:06:12 +0000 (UTC) X-FDA: 82907648166.13.41852BC Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) by imf11.hostedemail.com (Postfix) with ESMTP id EAB734001F for ; Wed, 18 Dec 2024 10:05:41 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=MmTr0Q+E; spf=none (imf11.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org; dmarc=none ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1734516338; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=LUQRqabp3as3yfp4jDRr00aDe4lMrHxwmyT4SmOcunE=; b=d1Zj1corkLWBGU9/uWME4DgIXNOkEpt4mBkWXDyjehLH5XLeFqZecjJ8AXYXgY5cfKxjAG LPRbL7tYh6KnDSLegoGVq70zd4Q4dTJ6DiNo9BedddvTyN+MOb+OoP9URmazbVDYZ9a34D PoyG6vhfrP0rrZzlNuEcIyjB9SRqg1s= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=infradead.org header.s=desiato.20200630 header.b=MmTr0Q+E; spf=none (imf11.hostedemail.com: domain of peterz@infradead.org has no SPF policy when checking 90.155.92.199) smtp.mailfrom=peterz@infradead.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1734516338; a=rsa-sha256; cv=none; b=lwuJwTBwlDKlX6dZ71+O+fCcmlPhJzFw7lP+2QLLD9miojMN9aOoHVBNMWe86472z5dFVI iUw7mugkE7mo3C3drsSqCmmv7qH3IvznytTAzuNRLbfisDNKSnXVPs5jh1zbT1S3vBlve4 1Z49vL0jVNBhP5VEsLOnI3/p8Ef/kmI= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=LUQRqabp3as3yfp4jDRr00aDe4lMrHxwmyT4SmOcunE=; b=MmTr0Q+EHZB4ClIHbKKZbY6dPK ePUIFS2Bwc1SNveCy4x1n7SL4ukY/s0QGfcfLP3Q4K1Vm7UDMaIl6MKjYE0WD2E2Az/mF+52ZnCDS imghNb6QtoQmQruZG1QOobt+DPHL/3p93XU8AVt96PkwDzfdraDMnxBiv23MGevotwFOG2ct9EXO/ W8rxQ65d7TxAnopaFGCA49O3ThQQcbLG/Q+v4ctmfNHUV/JPBGz3xxSaXHnseDpfbZELjmICq+hBU XbV8da4U6ko0VDyk0zWsgQ1irWJMRf86G+uHsz4NRGM/yrPb0nIl8tRBAlCj59vi+DNNYsAu0Anw8 Ub6L2y9A==; Received: from 77-249-17-89.cable.dynamic.v4.ziggo.nl ([77.249.17.89] helo=noisy.programming.kicks-ass.net) by desiato.infradead.org with esmtpsa (Exim 4.98 #2 (Red Hat Linux)) id 1tNqwL-00000005EEZ-2A25; Wed, 18 Dec 2024 10:06:01 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 128733002A0; Wed, 18 Dec 2024 11:06:01 +0100 (CET) Date: Wed, 18 Dec 2024 11:06:01 +0100 From: Peter Zijlstra To: Suren Baghdasaryan Cc: akpm@linux-foundation.org, willy@infradead.org, liam.howlett@oracle.com, lorenzo.stoakes@oracle.com, mhocko@suse.com, vbabka@suse.cz, hannes@cmpxchg.org, mjguzik@gmail.com, oliver.sang@intel.com, mgorman@techsingularity.net, david@redhat.com, peterx@redhat.com, oleg@redhat.com, dave@stgolabs.net, paulmck@kernel.org, brauner@kernel.org, dhowells@redhat.com, hdanton@sina.com, hughd@google.com, lokeshgidra@google.com, minchan@google.com, jannh@google.com, shakeel.butt@linux.dev, souravpanda@google.com, pasha.tatashin@soleen.com, klarasmodin@gmail.com, corbet@lwn.net, linux-doc@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, kernel-team@android.com Subject: Re: [PATCH v6 10/16] mm: replace vm_lock and detached flag with a reference count Message-ID: <20241218100601.GI12500@noisy.programming.kicks-ass.net> References: <20241216192419.2970941-1-surenb@google.com> <20241216192419.2970941-11-surenb@google.com> <20241216213753.GD9803@noisy.programming.kicks-ass.net> <20241217103035.GD11133@noisy.programming.kicks-ass.net> <20241218094104.GC2354@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20241218094104.GC2354@noisy.programming.kicks-ass.net> X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: EAB734001F X-Rspam-User: X-Stat-Signature: xmqhkiiurj1y8uoga5r89scpcyn9zf37 X-HE-Tag: 1734516341-91631 X-HE-Meta: U2FsdGVkX19WlnQc2vgi6/jVIdWiAUH2b5pM+Vv2nCyJJ4DYkIqeYzadH3cbfh6s87mieux8hCI99k6Up9JKi1Gjm/gPCxC6/w8Dh/oDV0SRi2ucy2cEwJ2Dk9O5CHXsCt/Vx4RmyjhShm9xiB0qeGYAOLiwIdedniA19nkm4eI+dQrLimMo0sbs/TXs3Iisz2TT5zgeNZLZ7k7YaQx6eZePqDfD0e4WaL3x52zqGb4CtLeZatUiKbWG8TlIGV407OEZtggt0b7hnIqvcejNjUUgsSJdscGNZ8foyAm3f8bV8iSrsogqxKNq7JrOoiDZjSRx5e/dnvotqLQJNmTZ1C0XrWgFyEXQwxdQoSFP1dmebZB2Z42bGMqGjjKN+iTJYjJr9QTLnekwuxBkARXlI+9KBdQ75XdkgbjDqzqudDmHm+aCwGEYN877gC2hFlEK01hI7lF6hrkO6aiwTnTVOW7UZHOEtD3216EOPxsgnN6S+1YcIdRW5MBkfLteZ+J/gC994+g5H15G0jPOlHNX8AJt2uTVAmoJk5tYGRrBmtKWO72rCUXmF1rxnE6mrsUzCWsRrnwvaOldFrgJ9QdBvJOQJY4ixSCc8Phez+5pbY4kppXA6O/KlxsfQ/OLmv7XAXViVpreSl/lWNzgBHpu/dnbkjTTeDlaRxbdyzVV6Zrh8x4sWT1nCx8REJfPGTZJlCOa6gphd5Ozhqlqi7rjJS7A/ylLInu2UUF6QbWjj8P5148QSbEjNwSairjwYGX3bjR2JbcAngXjxqmHom39S5ppmvzvtOLddzppP1ZLT/acIdlvrtgDNufLSbSafxlqptYSczR/b+VLXL4JtlkIaiznMfGD09GjYANeoE4SzFhDius89EpDeBWLoepTERrXRv1ATzanAKIQBueLnVnMjwZMJpdS0nI/0nzFHLoi5ZMO2LA8CBKIHa0/SaOtuhCK2KsS9dMfa/2PxDmnNW3 SljehPGY RNZlkym3QnjPAJJVA3mhBY81Pj6YriTw2t7OdJTHzhml84aal20ivxspg5LeDuXcWzArmVmYGurXAoV3aRdrJqY+/o1JQNx28yqqqNl36rGOhA8Kah/d41yRgU3RGkQbLcE8XehBqjDXELdkXxzKqC9F1iLWQeELPHq4+UI82hx8atUBbpupwI7oT8cC4FBft1RzpWXqxR+0qBty5U6FXQXuqU8su0VAMQc5r1z+sdJ4Yl9OD62+nXpsgmkWdcHjHLkFL/zI3I3xHrbMzJD7bHWfbV+UxgrhPEJFcN6DDEwQP/5o= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Dec 18, 2024 at 10:41:04AM +0100, Peter Zijlstra wrote: > On Tue, Dec 17, 2024 at 08:27:46AM -0800, Suren Baghdasaryan wrote: > > > > So I just replied there, and no, I don't think it makes sense. Just put > > > the kmem_cache_free() in vma_refcount_put(), to be done on 0. > > > > That's very appealing indeed and makes things much simpler. The > > problem I see with that is the case when we detach a vma from the tree > > to isolate it, then do some cleanup and only then free it. That's done > > in vms_gather_munmap_vmas() here: > > https://elixir.bootlin.com/linux/v6.12.5/source/mm/vma.c#L1240 and we > > even might reattach detached vmas back: > > https://elixir.bootlin.com/linux/v6.12.5/source/mm/vma.c#L1312. IOW, > > detached state is not final and we can't destroy the object that > > reached this state. > > Urgh, so that's the munmap() path, but arguably when that fails, the > map stays in place. > > I think this means you're marking detached too soon; you should only > mark detached once you reach the point of no return. > > That said, once you've reached the point of no return; and are about to > go remove the page-tables, you very much want to ensure a lack of > concurrency. > > So perhaps waiting for out-standing readers at this point isn't crazy. > > Also, I'm having a very hard time reading this maple tree stuff :/ > Afaict vms_gather_munmap_vmas() only adds the VMAs to be removed to a > second tree, it does not in fact unlink them from the mm yet. > > AFAICT it's vma_iter_clear_gfp() that actually wipes the vmas from the > mm -- and that being able to fail is mind boggling and I suppose is what > gives rise to much of this insanity :/ > > Anyway, I would expect remove_vma() to be the one that marks it detached > (it's already unreachable through vma_lookup() at this point) and there > you should wait for concurrent readers to bugger off. Also, I think vma_start_write() in that gather look is too early, you're not actually going to change the VMA yet -- with obvious exception of the split cases. That too should probably come after you've passes all the fail/unwind spots. Something like so perhaps? (yeah, I know, I wrecked a bunch) diff --git a/mm/vma.c b/mm/vma.c index 8e31b7e25aeb..45d43adcbb36 100644 --- a/mm/vma.c +++ b/mm/vma.c @@ -1173,6 +1173,11 @@ static void vms_complete_munmap_vmas(struct vma_munmap_struct *vms, struct vm_area_struct *vma; struct mm_struct *mm; + mas_for_each(mas_detach, vma, ULONG_MAX) { + vma_start_write(next); + vma_mark_detached(next, true); + } + mm = current->mm; mm->map_count -= vms->vma_count; mm->locked_vm -= vms->locked_vm; @@ -1219,9 +1224,6 @@ static void reattach_vmas(struct ma_state *mas_detach) struct vm_area_struct *vma; mas_set(mas_detach, 0); - mas_for_each(mas_detach, vma, ULONG_MAX) - vma_mark_detached(vma, false); - __mt_destroy(mas_detach->tree); } @@ -1289,13 +1291,11 @@ static int vms_gather_munmap_vmas(struct vma_munmap_struct *vms, if (error) goto end_split_failed; } - vma_start_write(next); mas_set(mas_detach, vms->vma_count++); error = mas_store_gfp(mas_detach, next, GFP_KERNEL); if (error) goto munmap_gather_failed; - vma_mark_detached(next, true); nrpages = vma_pages(next); vms->nr_pages += nrpages; @@ -1431,14 +1431,17 @@ int do_vmi_align_munmap(struct vma_iterator *vmi, struct vm_area_struct *vma, struct vma_munmap_struct vms; int error; + error = mas_preallocate(vmi->mas); + if (error) + goto gather_failed; + init_vma_munmap(&vms, vmi, vma, start, end, uf, unlock); error = vms_gather_munmap_vmas(&vms, &mas_detach); if (error) goto gather_failed; error = vma_iter_clear_gfp(vmi, start, end, GFP_KERNEL); - if (error) - goto clear_tree_failed; + VM_WARN_ON(error); /* Point of no return */ vms_complete_munmap_vmas(&vms, &mas_detach);