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 6A3E8C433EF for ; Wed, 15 Jun 2022 16:58:34 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 680D46B0074; Wed, 15 Jun 2022 12:58:33 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 62E876B0075; Wed, 15 Jun 2022 12:58:33 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 398F96B007B; Wed, 15 Jun 2022 12:58:33 -0400 (EDT) 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 202F66B0075 for ; Wed, 15 Jun 2022 12:58:33 -0400 (EDT) Received: from smtpin13.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id E64C334C41 for ; Wed, 15 Jun 2022 16:58:32 +0000 (UTC) X-FDA: 79581078864.13.BF8339F Received: from mail-pg1-f181.google.com (mail-pg1-f181.google.com [209.85.215.181]) by imf01.hostedemail.com (Postfix) with ESMTP id 78B874009F for ; Wed, 15 Jun 2022 16:58:32 +0000 (UTC) Received: by mail-pg1-f181.google.com with SMTP id 123so11908776pgb.5 for ; Wed, 15 Jun 2022 09:58:32 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=pCnfJCtja50aQCTQCL3tXCpA71s9K5w263ePUmZdP28=; b=hRYoXdpfjW2MUZxBSqtaHPrafkAxiKRBGeJjVxf/Sf1V1u6UBqtoJiwPWfEAnqDcwy o+uq988okfuv3GKvfMDMprmfucRsLfuFyXtMgi306xAtr4d34WX/h4Lj5jcf+pzmtlO3 +2nqd4Ojuk0WECWaGHZ5GBqjiKbcCt0lrL3GWJJeixzhYKAsKWR3ildaRN5VdgwBG10x ZJnxZvkqplZa4gbRQXOAS6TrAbwoA7j3G9vU2c18XfzmZslQKRzPhhRAhrUyAUIIUmcP 0F5bRbu429lw6B6YiA3MFJCW1hjYa5g9U+6hsqCTWMFAj3LGwp7xYPzJfHnsTqKZh2oE uhRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:subject:from:in-reply-to:date:cc :content-transfer-encoding:message-id:references:to; bh=pCnfJCtja50aQCTQCL3tXCpA71s9K5w263ePUmZdP28=; b=DObl5HXw50+98sSHORCk0atjGZBURP/tvOe58W/rK7TonYRaUa9CUD6RdAuuRRz6tg xFDhCIqNFcw2+3U60mCJIjZEJuTFDXrwAGDj8VdCKyivhEA0VhYhLFhuSo5wur8GRUmQ HNCRWqj793XHZj8IMqeHkAoTqptv2cSOMm0UvOumDkswET+J5aPyWiyQg0M37TgNbqS/ 31uVC5XWpCgFMvsAFvtjF5xkUbu49MAXAwHuZdi8EaErQpgp382Mf34IDsDxnswmrizd CXarIISb+hBHIdOJf5Uepx0YgzBMPTEl0FSlNFGYkK7Wu0UZv2y1dkeKdZsaBzmN/bBW oVEw== X-Gm-Message-State: AJIora/bZPM3T1/Nt0zSnBczJMyWAj4B9zUC/8PqznxbmMX9DqaVoiIL 7uEzoNkfgTPvrO7ixeqtwp8= X-Google-Smtp-Source: AGRyM1sFYkg2aoXqApahRtavLOtpT89zl3QEisi3NnHbIye/lKVgKjEOuV5MXg3FkmFJQz/1kxJrsQ== X-Received: by 2002:a05:6a00:319b:b0:51b:c5ec:62ba with SMTP id bj27-20020a056a00319b00b0051bc5ec62bamr526765pfb.1.1655312311127; Wed, 15 Jun 2022 09:58:31 -0700 (PDT) Received: from smtpclient.apple (c-24-6-216-183.hsd1.ca.comcast.net. [24.6.216.183]) by smtp.gmail.com with ESMTPSA id q8-20020a170902a3c800b0015e8d4eb22csm9552617plb.118.2022.06.15.09.58.29 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 15 Jun 2022 09:58:30 -0700 (PDT) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.100.31\)) Subject: Re: [PATCH RFC] userfaultfd: introduce UFFDIO_COPY_MODE_YOUNG From: Nadav Amit In-Reply-To: Date: Wed, 15 Jun 2022 09:58:27 -0700 Cc: Mike Rapoport , John Hubbard , David Hildenbrand , Linux MM , Mike Kravetz , Hugh Dickins , Andrew Morton , Axel Rasmussen Content-Transfer-Encoding: quoted-printable Message-Id: References: <20220613204043.98432-1-namit@vmware.com> <3eea2e6e-1646-546a-d9ef-d30052c00c7d@redhat.com> <481fc9d0-6122-bf59-9d04-23c10d256764@nvidia.com> <0BB58ACF-2801-4622-BF3B-9913A23AE46C@gmail.com> To: Peter Xu X-Mailer: Apple Mail (2.3696.100.31) ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655312312; a=rsa-sha256; cv=none; b=8UX9P0CcmShEGrdVC+DCbKiMF/aru40N2hWtUNimten/FsBsocV/9x1HK4l2MU4Fen2EGT pIfx6Vyqx2BChIfJrrl8mkZ0NsFm4Gf3fT+Nkvw8kdXa9XbkjRevF36ofIid78rQ5pm4e6 G34pZy2arHR00cTTcF2XCmfh9dfxvy4= ARC-Authentication-Results: i=1; imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=hRYoXdpf; spf=pass (imf01.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655312312; 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=pCnfJCtja50aQCTQCL3tXCpA71s9K5w263ePUmZdP28=; b=uk+2VO3UO9GJMliy5oL7T4gkW0LkJ9SKmSiDritLK7kYXs0LXgckD3Y9La66DCXgEehGSj GHVsqvERenKNZg247TlN1nOJDZSjdzwWDGrKcZKTNCTBu9MbpR9liX6mmqX5LyfStRDaZx +KSauPvZN+YMfQb7ClKH6GviQFrqB5Y= X-Stat-Signature: 88djdp74am39mfxngy9eqeojzhru9rfs X-Rspamd-Queue-Id: 78B874009F Authentication-Results: imf01.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=hRYoXdpf; spf=pass (imf01.hostedemail.com: domain of nadav.amit@gmail.com designates 209.85.215.181 as permitted sender) smtp.mailfrom=nadav.amit@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspam-User: X-Rspamd-Server: rspam02 X-HE-Tag: 1655312312-632781 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: On Jun 15, 2022, at 8:43 AM, Peter Xu wrote: > On Wed, Jun 15, 2022 at 10:26:21AM +0300, Mike Rapoport wrote: >> On Tue, Jun 14, 2022 at 01:56:56PM -0700, Nadav Amit wrote: >>> On Jun 14, 2022, at 1:40 PM, John Hubbard = wrote: >>>=20 >>>> On 6/14/22 11:56, Mike Rapoport wrote: >>>>>> But, I cannot take it anymore: the list of arguments for uffd = stuff is >>>>>> crazy. I would like to collect all the possible arguments that = are used for >>>>>> uffd operation into some =E2=80=9Cstruct uffd_op=E2=80=9D. >>>>> Squashing boolean parameters into int flags will also reduce the = insane >>>>> amount of parameters. No strong feelings though. >>>>=20 >>>> Just a quick drive-by comment about boolean arguments: they ruin = the >>>> readability of the call sites. In practice, sometimes a single = boolean >>>> argument can be OK-ish (still poor to read at the call site, but = easier >>>> to code initially), but once you get past one boolean argument in = the >>>> function, readability is hopeless: >>>>=20 >>>> foo(ptr, true, false, a =3D=3D b); >>>>=20 >>>> So if you have a choice, I implore you to prefer flags and/or = enums. :) >>>=20 >>> Thanks for the feedback - I am aware it is very confusing to have = booleans >>> and especially multiple ones in a func call. >>>=20 >>> Just not sure how it maps to what I proposed. I thought of passing = as an >>> argument reference (pointer) to something similar to the following = struct, >>> which I think is very self-descriptive: >>>=20 >>> struct uffd_op { >>> /* various fields */ >>> struct vm_area_struct *dst_vma; >>> unsigned long len; >>> atomic_t *mmap_changing; >>>=20 >>> ... >>> =09 >>> /* ... and some flags */ >>> int wp: 1; >>> int zero: 1; >>> int read_likely: 1; >>>=20 >>> ... >>> }; >>>=20 >>> I think that fits what you were asking for. The only thing I am not = sure of, >>> is whether to include in uffd_op fields that are internal to = mm/userfaultfd >>> such as =E2=80=9Cpage=E2=80=9D and =E2=80=9Cnewly_allocated=E2=80=9D. = I guess not. >>=20 >> mfill_atomic_install_pte() is called by shmem_mfill_atomic_pte() so = it's >> not entirely internal to mm/userfaultfd.c. >>=20 >> Another thing is that with all the parameters packed into a struct, = the >> call sites could become really hairy, so maybe the best way would be = to >> pack some of the parameters and leave the others. >>=20 >> But you'll never know until you try :) >=20 > Yeh. Axel packed some booleans in f619147104c8e into = mcopy_atomic_mode. > The other option (besides uffd_ops) could be making mcopy_atomic_mode = a > bitmask and keep the rest, the mode itself only took 2 bits. >=20 > uffd_ops sounds good too if the final outcome looks clean, since we do = pass > quite a few things over and over deep into the stack. Thanks. I see 3 options: 1. Pack only fs/mm flags: WP, read-likely, write-likely. 2. (1) + as part of the flags internally include Axel=E2=80=99s = copy_atomic_mode. 3. The uffd_op approach: include all relevant fields. For the time being I=E2=80=99m going with (1) since I do not have too = much time to finish all of that and upstream the rest of my work (Broadcom is = knocking). (3) also has the downside of stack-protector that would be added due to stack-protector strong, which is not-that-bad, but I hate it. Three more points for consideration in future cleanups: 1. This __always_inline thingy is crazy IMHO. The size of the = compilation unit is almost double because of it, and I saw no explanation for its = use in the commit log (unless I missed it). The overheads in userfaultfd are = mostly due to memory copying, scheduling, IPIs. 2. I think it makes more sense to strive not to have more than 6 = arguments for each function (as supported in registers on x86). For that it is = possible to get rid of dst_mm when it can be retrieved from dst_vma. Anyhow we = access dst_vma->vm_flags which share a cache-line with dst_vma->vm_mm. 3. These BUG_ON()s all around are also ... excessive. I guess they were introduced before the age in which Linus got angry on each BUG_ON(). Is there any good reason not to change them into VM_BUG_ON()?