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 34E39C3ABCC for ; Tue, 13 May 2025 15:31:53 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 1B1726B00BD; Tue, 13 May 2025 11:31:52 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 15F1B6B00BE; Tue, 13 May 2025 11:31:52 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id F1BF16B00D0; Tue, 13 May 2025 11:31:51 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id D179B6B00BD for ; Tue, 13 May 2025 11:31:51 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 0567AC06C6 for ; Tue, 13 May 2025 15:31:52 +0000 (UTC) X-FDA: 83438274864.13.4ECE6E4 Received: from mail-qt1-f176.google.com (mail-qt1-f176.google.com [209.85.160.176]) by imf27.hostedemail.com (Postfix) with ESMTP id 293A04000A for ; Tue, 13 May 2025 15:31:49 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=WDzEUvg1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf27.hostedemail.com: domain of surenb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1747150310; a=rsa-sha256; cv=none; b=X3zPQPBtW2m6o7uVaAitiiAbR+EyTHjFgorLlsAdyQNKNvvrwMs62VaZHFQZA62GYbNuul EyG1f6YLs2w03grZYDW7Sq7ZMEUMh7wZkpoECUD+v9QM0mShed3lCzNE60AQzVaR4DAM5J zLAgwXhLnXKH7NFlabDaQEqPpO2AXOc= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=pass header.d=google.com header.s=20230601 header.b=WDzEUvg1; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf27.hostedemail.com: domain of surenb@google.com designates 209.85.160.176 as permitted sender) smtp.mailfrom=surenb@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1747150310; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=/pcbAFEUe6CndbNveQVJGhzDrvXg2EyJ/NUFQzfb4yY=; b=SlqAnIUHf+YHDwtdjbvO1KFWvTPqIbKXfek1hh3qewppZOQSAOXdq/Qf9VouPma9fFpnm0 z0MXprx/mffYvuJ1xCTR6PeXRbICF8t5nrSnCvoeEzbR93k6QmNYYLxQoSkFjxPnbEPyEJ 3oz6PzaopHiKAnG8hPGuV0bJR8vNoUU= Received: by mail-qt1-f176.google.com with SMTP id d75a77b69052e-47666573242so398181cf.0 for ; Tue, 13 May 2025 08:31:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1747150309; x=1747755109; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=/pcbAFEUe6CndbNveQVJGhzDrvXg2EyJ/NUFQzfb4yY=; b=WDzEUvg16y9sxb2IPvP+PeyS4mUG/uubwNXAn5TH8rSFh592Y2RWgr0G64YV5dcUnr X/uUwm3pvs67LZxvDh6V4yFtNNi7qrUZrXW/qh09ZImMqHEpuOl4CWtS1ct3E0HWswFz wu2kUTtQEZIi2vbPCQHJ3ItmZxVF8KAiD8CjPEb/x12hkqY4LFAuiPNFApZdPcYQu0wd pmWbRO/FQnHHJyEJDfMRW1TtRZZxOkFDBzh+KmqmFny0qgrrRvTuLQmQUlPJtCidjuww WiIpOI2Rv7D/jsM7RRlge8PZRIAebeju29Q+ObPJubZuygDRdl8ZQPKkvffQT4ImARq0 XLxw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1747150309; x=1747755109; h=content-transfer-encoding:cc: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=/pcbAFEUe6CndbNveQVJGhzDrvXg2EyJ/NUFQzfb4yY=; b=qsUzCcaOxNlrTomC7asBEHka1dfCh954TtUl18makCCdoSfSx2KfWZ1jx6pdRH4FF7 bQydI47T5BzjMWtA1gsWx3b9a3+fIlKkTgCs9PhIUf5sJwFqNfUAFQYgoogwgeTgZwSe PEVUeeoPWvlqK0P/hdyGKGP6AjPoWOTlAnWrqLvaJqFevtAYcnVz1MER2Nb+VqUWgfjN Wwv5tCFZu28zvztun0ojLUR/DjgeKQht16iutASbUYySo0OUTUCoI0cUB/jwCJIV7kYd /QKP7ThzGG8q8kKzrbvWc5PDDboMF5TF2cyPJKhmL3IB0eubPnPy9JVUDZvJZv9i54Xw 2yYQ== X-Forwarded-Encrypted: i=1; AJvYcCWdm/BWaYK0K9SVEyiTKdLWgMUfgFNuhxbITRwTsPmuq8i+28RZoPv6OV8B9+OFJGb1KIdMUVbxcA==@kvack.org X-Gm-Message-State: AOJu0YzjSqN3gU+XY+ck8u/021rvhQ+mkmB8IQdEUtpkgzIZ9jaOwrCD YedVQpkQLnfuAtdpPggfixGXgrat4BUp9wT/lqAHonCU7WqwfkoPOA9zs07n4atAfL8uNJtEu32 eDxmLoK2nX8JQnB3LaWb5EybPyNn4Hx0mbfWdKbYQ X-Gm-Gg: ASbGnctpKKoLtkTyTM2lqWkIwVsM9aCDgVP/t1xxl1LrPx4XivO6elZAmE3/UIe9Q1K LivE5ZbLnRWDY1OXHCvolx+XKag46m0z2JP2KSLyB/npXRLJ0NoR87ZihkJWBUiRa4V+XWCkLo7 EnncqtmxLlZkC4EVhsu91390xGY+T4miT2/if/SB68rW+hwr6a88MN7o82IhYe7dVa9It4VZg= X-Google-Smtp-Source: AGHT+IEazOLSY/I5yGsaKVwi1YOFrm7Vx4mUNOPA4wc5ctfib+E7WUz0Gs1e/wYgWOti4hvr1qmtAH/BNX0+MWB6wSM= X-Received: by 2002:a05:622a:202:b0:471:eab0:ef21 with SMTP id d75a77b69052e-494898efbb9mr4152841cf.13.1747150307319; Tue, 13 May 2025 08:31:47 -0700 (PDT) MIME-Version: 1.0 References: <06e2e29d-20c7-4999-b36b-343cf083f766@redhat.com> In-Reply-To: <06e2e29d-20c7-4999-b36b-343cf083f766@redhat.com> From: Suren Baghdasaryan Date: Tue, 13 May 2025 08:31:35 -0700 X-Gm-Features: AX0GCFuAMnahTxKUQ5UF1UB4gu8J9u13u9DtXFbpl14HvEiDuGsfXLXRvl8vNz4 Message-ID: Subject: Re: [PATCH v2 1/3] mm: introduce new .mmap_prepare() file callback To: David Hildenbrand Cc: Lorenzo Stoakes , Andrew Morton , "Liam R . Howlett" , Vlastimil Babka , Mike Rapoport , Michal Hocko , Jann Horn , Pedro Falcato , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Alexander Viro , Christian Brauner , Jan Kara , Matthew Wilcox Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 293A04000A X-Stat-Signature: q17i3qsm3hbbx7qhdy8oga6mbhxz85rt X-HE-Tag: 1747150309-674381 X-HE-Meta: U2FsdGVkX1/2v7nMfh7WrToIc0T6KX6GiqCD0naglSAxeqVq4Ml0wEnq4E79H/i0wmnRX3uICQZn+wmPHCc6IMxd9ZL5DxdpTOfu5s9VKLkNVcXiYmMKpG34quiG1JTnzJh1/01mN3GWQhIhBF5aHmjikIW9WLlCpGAcXT8KDe5Pu4EMAjMYyf85yHopamWu0ZZoZybAPcrJ8J29lPORKhDYwQTOePbtgDW8kKWVZ/3Mfl2l/KPyU+uRt4OxOJE/HPW17Yc8Qs6wDuUnTGc7snyWIOf97+PjPA54l/mAI0ZevP+g8wsA6mEnf5TD3nNv0zfY3Gfh7EqmSioplJdXAUUHr5H5MxC0V0IISSbcsKKHwCfzk8XZNGk8gDtAob8V0CvxgVGC7lkgcrJeKyUtTZzMDBGZ/4+W/Wh09LOwYPuN7oRvwq8IUDUDuxJ5k9YLOuhA9ncENhynL+xjA2ZVsWy/wKsilJz0CLhqBgDxF8VFwbwcApXPUEcsKEQ7NZGGeZHP2R9u60AGTeiLgq3kxrvQ3Lp1XVWs4fHWE9xIT4R65Fan43jqoDWVnDYoFkbN+t8R5NqrHq8bFTIrT3m6s355AhPvcNKljiiLAdTCW6l+1pVBVtH7oeNrGYA9gPebnp5ZHfAxPUsoH3gdN5+z/cmy4cUJBBdV2S1dwI/7try2pc5gAs2ryGwvmSYlvdj1qimzTRu3SBpuvLxmw1cu63zEDLPVbFfnIAJgR8Vwgb9GNjcjROIwMthntY3qJnb5BcUQPtLuqq03mXEh1uq7PKa+Ef14mxeaVNDPwndUB+JPuGT5XhRP5D1YH7vElic0UGNMtNSeIwRcijMbHBcKsf9RMmJcOVB4FLTFf7V/4H4IJpAvm2dIvZk1LxQvqWohd4gJv+rn46spq/TYeUW0KO3orsxlT75kIiTVbHQmlxPS0ZYkUZlLOsYio9UnRpglOvc+rNlMBoWZSGs+xuZ KqXW26Qg DN8Vt8FH2rksLOciv/6lcyFJC0PD14VYYnEnEUX7AEeabWeVR17kNLwQeeWc9Wckpv+M4JW4EQSdz7n5r6VM//AYYJngxfXINX6rHsOSpK0y9awipUB1qJF9cAaMo4TP6KXywLzTffGSzEQfN/9t99QrDGOjDkZZWCWtmjKqXA1iNynplz+bfV0MkFr9FaAJYyQk9uTgSv4dLcU3IGhIJGvy242sQRKOCdRHp4RVOOSg5/+3orrqA4qBzjgwykQz9wluUIi+S979Jrdmdm9lLhAGiM48BL9k4pokVMy4m2mSzMXvu9dUrtKGFLw== 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, May 13, 2025 at 6:25=E2=80=AFAM David Hildenbrand wrote: > > On 13.05.25 11:32, Lorenzo Stoakes wrote: > > On Tue, May 13, 2025 at 11:01:41AM +0200, David Hildenbrand wrote: > >> On 09.05.25 14:13, Lorenzo Stoakes wrote: > >>> Provide a means by which drivers can specify which fields of those > >>> permitted to be changed should be altered to prior to mmap()'ing a > >>> range (which may either result from a merge or from mapping an entire= ly new > >>> VMA). > >>> > >>> Doing so is substantially safer than the existing .mmap() calback whi= ch > >>> provides unrestricted access to the part-constructed VMA and permits > >>> drivers and file systems to do 'creative' things which makes it hard = to > >>> reason about the state of the VMA after the function returns. > >>> > >>> The existing .mmap() callback's freedom has caused a great deal of is= sues, > >>> especially in error handling, as unwinding the mmap() state has prove= n to > >>> be non-trivial and caused significant issues in the past, for instanc= e > >>> those addressed in commit 5de195060b2e ("mm: resolve faulty mmap_regi= on() > >>> error path behaviour"). > >>> > >>> It also necessitates a second attempt at merge once the .mmap() callb= ack > >>> has completed, which has caused issues in the past, is awkward, adds > >>> overhead and is difficult to reason about. > >>> > >>> The .mmap_prepare() callback eliminates this requirement, as we can u= pdate > >>> fields prior to even attempting the first merge. It is safer, as we h= eavily > >>> restrict what can actually be modified, and being invoked very early = in the > >>> mmap() process, error handling can be performed safely with very litt= le > >>> unwinding of state required. > >>> > >>> The .mmap_prepare() and deprecated .mmap() callbacks are mutually > >>> exclusive, so we permit only one to be invoked at a time. > >>> > >>> Update vma userland test stubs to account for changes. > >>> > >>> Signed-off-by: Lorenzo Stoakes Reviewed-by: Suren Baghdasaryan > >>> --- > >>> include/linux/fs.h | 25 ++++++++++++ > >>> include/linux/mm_types.h | 24 +++++++++++ > >>> mm/memory.c | 3 +- > >>> mm/mmap.c | 2 +- > >>> mm/vma.c | 68 ++++++++++++++++++++++++++++= +++- > >>> tools/testing/vma/vma_internal.h | 66 ++++++++++++++++++++++++++++= --- > >>> 6 files changed, 180 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/include/linux/fs.h b/include/linux/fs.h > >>> index 016b0fe1536e..e2721a1ff13d 100644 > >>> --- a/include/linux/fs.h > >>> +++ b/include/linux/fs.h > >>> @@ -2169,6 +2169,7 @@ struct file_operations { > >>> int (*uring_cmd)(struct io_uring_cmd *ioucmd, unsigned int issue_= flags); > >>> int (*uring_cmd_iopoll)(struct io_uring_cmd *, struct io_comp_bat= ch *, > >>> unsigned int poll_flags); > >>> + int (*mmap_prepare)(struct vm_area_desc *); > >>> } __randomize_layout; > >>> /* Supports async buffered reads */ > >>> @@ -2238,11 +2239,35 @@ struct inode_operations { > >>> struct offset_ctx *(*get_offset_ctx)(struct inode *inode); > >>> } ____cacheline_aligned; > >>> +/* Did the driver provide valid mmap hook configuration? */ > >>> +static inline bool file_has_valid_mmap_hooks(struct file *file) > >>> +{ > >>> + bool has_mmap =3D file->f_op->mmap; > >>> + bool has_mmap_prepare =3D file->f_op->mmap_prepare; > >>> + > >>> + /* Hooks are mutually exclusive. */ > >>> + if (WARN_ON_ONCE(has_mmap && has_mmap_prepare)) > >>> + return false; > >>> + if (WARN_ON_ONCE(!has_mmap && !has_mmap_prepare)) > >>> + return false; > >>> + > >>> + return true; > >>> +} > >> > >> So, if neither is set, it's also an invalid setting, understood. > >> > >> So we want XOR. > >> > >> > >> > >> const bool has_mmap =3D file->f_op->mmap; > >> const bool has_mmap_prepare =3D file->f_op->mmap_prepare; > >> const bool mutual_exclusive =3D has_mmap ^ has_mmap_prepare; > >> > >> WARN_ON_ONCE(!mutual_exclusive) > >> return mutual_exclusive; > > > > Yeah I did consider xor like this but I've always found it quite confus= ing > > in this kind of context, honestly. > > With the local variable I think it's quite helpful (no need for a > comment :P ). > > > > > In a way I think it's a bit easier spelt out as it is now. But happy to > > change if you feel strongly about it? :) > > Certainly not strongly! :) > > -- > Cheers, > > David / dhildenb >