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 E86FAC48297 for ; Tue, 6 Feb 2024 16:26:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6B26B6B0078; Tue, 6 Feb 2024 11:26:53 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 6628C6B007B; Tue, 6 Feb 2024 11:26:53 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 52BE56B007D; Tue, 6 Feb 2024 11:26:53 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 4294E6B0078 for ; Tue, 6 Feb 2024 11:26:53 -0500 (EST) Received: from smtpin01.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id C6B701A0997 for ; Tue, 6 Feb 2024 16:26:52 +0000 (UTC) X-FDA: 81761907864.01.FB01C5D Received: from mail-wr1-f46.google.com (mail-wr1-f46.google.com [209.85.221.46]) by imf11.hostedemail.com (Postfix) with ESMTP id 08EFF4000C for ; Tue, 6 Feb 2024 16:26:50 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ggJQ0OAS; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of lokeshgidra@google.com designates 209.85.221.46 as permitted sender) smtp.mailfrom=lokeshgidra@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707236811; a=rsa-sha256; cv=none; b=va3wCTFVVPsnkH0ri3rGYiZ/0w6TVS0EOiUys2DCX+iJd124F2izpMWSbu2TmxDqCmSi8j r4loUHWWuTFrKHlK0adlt4o1+dY7OpcMABrQ7Kq2MDPlbpCDqifHEiIpPJ3Q2oTrvNLF0d IDrFPUZ2TCHiOVp6lYj7MjxB/PidHGE= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=ggJQ0OAS; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf11.hostedemail.com: domain of lokeshgidra@google.com designates 209.85.221.46 as permitted sender) smtp.mailfrom=lokeshgidra@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707236811; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=oKcGartFfaDNzaw0oNAzfboMjdH4dZ1SThMOYPfs59E=; b=tJJZ228KU7dYjO9w0TEt11+ufLwSLkMhihS+f7v2c6uY04rEt4FWFznHVo4S13isNzNPfT dbK02SYufHRMJc9A1/J/OHqDTn2WogORY2FOGhWC4shV3oL8R20NEZHVz+cCx+UbVKC9ep iFxfwWlUzzLWFGrU+Z5cUeJWQg1K680= Received: by mail-wr1-f46.google.com with SMTP id ffacd0b85a97d-33b29b5ea86so2363539f8f.1 for ; Tue, 06 Feb 2024 08:26:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1707236809; x=1707841609; darn=kvack.org; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=oKcGartFfaDNzaw0oNAzfboMjdH4dZ1SThMOYPfs59E=; b=ggJQ0OASe7Naew/jceWrCRDWLdaWHjYZCZikr6B70ae/EXIrzdTCaDQCj7lkugfqQb 558rvJw2xTwYI7+xmZ5hZ8si5DfFwNGe2DMOvTIC6ui1PCFnXtGqThlz/SzDrqKPEgUf gPume9Y5jjvimRXgWE6h6QKFaJ+WdbOx2s2nE8K+HB46+Qp3CsbaEMokCPPYXU/2YRCo vZchMNF1mU0JJ9ZVw15zcp3v+ikFk3PMgF90XenwGwYWyFKBHYdA9YRxwBgSn1fORHPl G0eatuMSSgCS/SlxLP/3/a6iK/4bMdQW9C8SCA2cWIS1hNEggh19c0yaGCgSd4AIFId4 b4Ag== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707236809; x=1707841609; h=content-transfer-encoding:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=oKcGartFfaDNzaw0oNAzfboMjdH4dZ1SThMOYPfs59E=; b=dqE9j0iRslrJJUFX+wKbyKOaBegwoSuRnkU+tFGmf1oQhz994Jx7bQJXC0rUadWr1V 3O7ACTfiP8gSra3yA27l/pB21DibxV1xiBodMXAEbMnDVomUWpLqLpUiBrmjW0GmLEO0 iyYMZhW/xqb0ClW53H/ZBdd6SVF5PEdNCRtcDTq3BmaMzUgQeLyvoF9glPD1iCyFbSak YEywwNY63fsEaj6uAthIVoOuvcJtbIRE376Bags1P3LkEP0N3TMpbiR9cuLWT18sVin+ YMZOuWuhIE5Pg6HbCvSBFPkwjkYkyF7YPxnFux3ppw4kHBG+QuPJHqoKjYRVqAqw5QOG 1QsQ== X-Gm-Message-State: AOJu0YzGcgAsCPePwZESpy1Ry3C6Z1X75up54JthSQe8/xRYU/7dg6MB kh4Uc07uoBnCuHYmB/8rhIyIXZiOw2eHmc9AZ09eScL8bV7Mlux1KxSKKO0LVRWqv09xMEc3btZ UB6ByyGq+QhZOm2yFt5kC1S3H00+xTZVaWxbf X-Google-Smtp-Source: AGHT+IH89GUBiASlkHU1t/vbfYCWBR4/w2Ue/VOkFZNVrEQU2R/i0VQYZk26ng6YoRY9yOTsCfq4LDgm9PGa95ZnLvE= X-Received: by 2002:a5d:6d87:0:b0:33b:48ef:22ed with SMTP id l7-20020a5d6d87000000b0033b48ef22edmr1605036wrs.35.1707236809234; Tue, 06 Feb 2024 08:26:49 -0800 (PST) MIME-Version: 1.0 References: <20240129203626.uq5tdic4z5qua5qy@revolver> <20240130025803.2go3xekza5qubxgz@revolver> <20240131214104.rgw3x5vuap43xubi@revolver> <20240205220022.a4qy7xlv6jpcsnh7@revolver> <20240206143506.6zsj2gktu754gvl6@revolver> In-Reply-To: <20240206143506.6zsj2gktu754gvl6@revolver> From: Lokesh Gidra Date: Tue, 6 Feb 2024 08:26:36 -0800 Message-ID: Subject: Re: [PATCH v2 3/3] userfaultfd: use per-vma locks in userfaultfd operations To: "Liam R. Howlett" , Lokesh Gidra , Suren Baghdasaryan , akpm@linux-foundation.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, selinux@vger.kernel.org, kernel-team@android.com, aarcange@redhat.com, peterx@redhat.com, david@redhat.com, axelrasmussen@google.com, bgeffon@google.com, willy@infradead.org, jannh@google.com, kaleshsingh@google.com, ngeoffray@google.com, timmurray@google.com, rppt@kernel.org Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 08EFF4000C X-Stat-Signature: 413n4yf64arix4efkmqy54u7s1gpmn3s X-HE-Tag: 1707236810-378441 X-HE-Meta: U2FsdGVkX19GDlH7fU2yLKygbCoYDrbFBvaoE8wfjUnwN0Mua6kYf6D2nfmb178eJyGTXL+IG3KBcmVbzmgkg7TDCNaF0PQU4+8CcnNsDZ+GrgDFZHovCNd8+PImdmH1zNQnlInwjaeGJBlyoBOWz7MFpKrY5IQKp+UV2dZGnYjs86ew333lpdq+nurzMXcvix3d5+v7rxXBnkpHZF9jm7ILO8mhxA7lAl9LvHQzjNJy1SaWaMm7pFIQ468Oap5AHkwPPtPLeZ/PHcKGXzLUUb4g7JPQqKLowaoexAjDDO8RfQu4kV2OqGjPmPoLvru3if8zyBF35x7t8froCQSyDAJho4T/1J10MprByUMmeQ95gzd8CjQwXJ1unwXfy8cgFom7fTuZWm7IvYUQRljXTv+9HXVuzY2pd5eZ4qIyZnv8HzTOSkqtI1++qmrS2fUtTRQZcNAQ4007F7NXZ90K8AWmwY/s3mUAeH7rFhcvhNQo/4MbsuFxj2foa4t93Blj1o+TRnqGgP/BbGmo8eDtgVIXc7+c5y71BuqgCkE3W5aa/BbYzEr0A+vOt8HbWMLiMu8MzNgvu7dntKcQPOpGceexduzoStGifhGhplx37E5n11w4+SDNHVuL7wI91tVn2w9nF5y7qRJvtKqVs80B6Eda+ro05eLfrMv9mj37beRxMbVj5xZhTQeu08C8xk000BVdJTeiZ5n1+NyRDry8UFDHX8dtmAPhe/Rkm7w9HiL86i6AXzVHZNVxRDTZMu+0+rP13MZONWibTlCX48H0++lZrUIc+5mWi2veLmXpPWKKP35zubxwn2SJ4dNB3trzAgSjC5nCf+aD0ztRA0xHnwEDsL+72zWk03d2rlG4z3UoE0ujhvdTfTpAUVHEF3ve4J/akjxQx6Hlbi6RderHGfCMA6duAjTqz8ID8UeXQfYXAXdkjuDDkRl1Dg9S6lLwTx1GcNhauM+kX6iDUg8 OIB4SVU2 Mw7lSgT+7cM3uehmr4ermTZWuW165gOStbq+bVn3RAx8SzrKrhvrLlJ35nmWw9+TXv8QFRZFmA45nnBu16MwsBhsxsN2u3TJsEvoivMdDZtBDYbNnXNitWMqAF8HXfV6TkbJvJh5U5p3agPzqp6cc4xCimQq7NwypEgPasptPQ7BYOatgvraatTg6u8glL4YSV1ZkZZ1po5hku+OLJBxbboo97rkNEwmNdxufQV0kpyvlm1KDLugIuoojyxm1r0RuZD3oSdhNmCaAUwxeXKpmWyzXOQ== 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 Tue, Feb 6, 2024 at 6:35=E2=80=AFAM Liam R. Howlett wrote: > > * Lokesh Gidra [240205 17:24]: > > On Mon, Feb 5, 2024 at 2:00=E2=80=AFPM Liam R. Howlett wrote: > > > > > > * Lokesh Gidra [240205 16:55]: > > > ... > > > > > > > > > > We can take care of anon_vma as well here right? I can take a= bool > > > > > > > parameter ('prepare_anon' or something) and then: > > > > > > > > > > > > > > if (vma) { > > > > > > > if (prepare_anon && vma_is_anonymous(vma)= ) && > > > > > > > !anon_vma_prepare(vma)) { > > > > > > > vma =3D ERR_PTR(-ENOMEM= ); > > > > > > > goto out_unlock; > > > > > > > } > > > > > > > > vma_aquire_read_lock(vma); > > > > > > > } > > > > > > > out_unlock: > > > > > > > > mmap_read_unlock(mm); > > > > > > > > return vma; > > > > > > > > } > > > > > > > > > > > > Do you need this? I didn't think this was happening in the cod= e as > > > > > > written? If you need it I would suggest making it happen alway= s and > > > > > > ditch the flag until a user needs this variant, but document wh= at's > > > > > > going on in here or even have a better name. > > > > > > > > > > I think yes, you do need this. I can see calls to anon_vma_prepar= e() > > > > > under mmap_read_lock() protection in both mfill_atomic_hugetlb() = and > > > > > in mfill_atomic(). This means, just like in the pagefault path, w= e > > > > > modify vma->anon_vma under mmap_read_lock protection which guaran= tees > > > > > that adjacent VMAs won't change. This is important because > > > > > __anon_vma_prepare() uses find_mergeable_anon_vma() that needs th= e > > > > > neighboring VMAs to be stable. Per-VMA lock guarantees stability = of > > > > > the VMA we locked but not of its neighbors, therefore holding per= -VMA > > > > > lock while calling anon_vma_prepare() is not enough. The solution > > > > > Lokesh suggests would call anon_vma_prepare() under mmap_read_loc= k and > > > > > therefore would avoid the issue. > > > > > > > > > > > ... > > > > > > > anon_vma_prepare() is also called in validate_move_areas() via move= _pages(). > > > > > > Probably worth doing it unconditionally and have a comment as to why = it > > > is necessary. > > > > > The src_vma (in case of move_pages()) doesn't need to have it. > > > > The only reason I'm not inclined to make it unconditional is what if > > some future user of lock_vma() doesn't need it for their purpose? Why > > allocate anon_vma in that case. > > Because there isn't a user and it'll add a flag that's a constant. If > there is a need for the flag later then it can be added at that time. > Maybe there will never be a user and we've just complicated the code for > no reason. Don't implement features that aren't necessary, especially > if there is no intent to use them. > I'm not too attached to the idea of keeping it conditional. But I have already sent v3 which currently does it conditionally. Please take a look at it. Along with any other comments/changes that I get, I'll also make it unconditional in v4, if you say so. > > > > > Does this avoid your locking workaround? > > > > Not sure which workaround you are referring to. I am almost done > > implementing your suggestion. Very soon will share the next version of > > the patch-set. > > The locking dance with the flags indicating if it's per-vma lock or > mmap_lock. > That dance was not because of anon_vma. It's just that I hadn't realized that we can do it the way you suggested :) I really liked your suggestion and is implemented in v3. PTAL.