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 D006BC43334 for ; Tue, 21 Jun 2022 08:48:59 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6D3436B0072; Tue, 21 Jun 2022 04:48:59 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 683E96B0073; Tue, 21 Jun 2022 04:48:59 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 524088E0001; Tue, 21 Jun 2022 04:48:59 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 3D7D26B0072 for ; Tue, 21 Jun 2022 04:48:59 -0400 (EDT) Received: from smtpin27.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 15C2620E8A for ; Tue, 21 Jun 2022 08:48:59 +0000 (UTC) X-FDA: 79601617998.27.248410D Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf17.hostedemail.com (Postfix) with ESMTP id A6FE7400AC for ; Tue, 21 Jun 2022 08:48:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1655801338; h=from:from: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; bh=w9DrM7fA/Vr1/hLg7C5EtTv+FJvTNPajneiswujjz00=; b=NHNH4sqd4kbwuEw/sigWnQtdjHVUfF7oE04mmgFMinejXXrqyOt+JZb2pkKa1kWFjUz9RF TCKKfJDoOUZ/yk02y14fwcHZAacVUAtdOSVVJnT2xplFwI2upcN1GI17x3ORFT7rQi0FEI IOcVJ17ncJlvOP6tC4ODKlmjP4V7PQ8= Received: from mail-wr1-f72.google.com (mail-wr1-f72.google.com [209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-638-UmkZ4VgtPB6JATtcX6j1Wg-1; Tue, 21 Jun 2022 04:48:54 -0400 X-MC-Unique: UmkZ4VgtPB6JATtcX6j1Wg-1 Received: by mail-wr1-f72.google.com with SMTP id y18-20020adfdf12000000b0021b94ba4c37so690646wrl.11 for ; Tue, 21 Jun 2022 01:48:54 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent :content-language:to:cc:references:from:organization:subject :in-reply-to:content-transfer-encoding; bh=w9DrM7fA/Vr1/hLg7C5EtTv+FJvTNPajneiswujjz00=; b=Mr9N2WTxfL+U+ayjrkng3yh7udn4OVPnjjJuERtirpELNWa+FbO09yzfVm5keK5mR9 KrCfO4FvQJFoFfVzMzZoH4284xL1MD/JK8mwALdAacdRtN2JG/MRMH9/CUj11P8Ic36y VnuD3v6G2hsnowOe1ZVsnKwB9+R4vKg/r8pIhvrH2bgz/Bsfsz8CRMu/8j3w//3WNWxL GJRJbGCrSuEktpQDe7Y7uLx/aNPwBATUa41p6C5m60nwln4VusIbDld1rDtUAraXJ2md /oLQaNODNT15iXXn32AjcFmyeWDIOYjFBgp5Te1fTnLMxKzVJSrhvQH0+29kbf7sFwmF Dn9Q== X-Gm-Message-State: AJIora9D50tnjxdxj0S3neUpyIUzjv8wKc4SL3pcn2FkO+sGpBxbtqKf BwMpeNVHJesTShunn/rkMfV+yxzeLA8GQGkZBZisKPYAbI5MUi0EaZed+CFeM3Y+hcfz7eV7yAH Z0nRSF1TfpBY= X-Received: by 2002:a5d:6c6b:0:b0:1ea:77ea:dde8 with SMTP id r11-20020a5d6c6b000000b001ea77eadde8mr27428815wrz.690.1655801333113; Tue, 21 Jun 2022 01:48:53 -0700 (PDT) X-Google-Smtp-Source: AGRyM1uAIqT1uZkLht9LNqN1dB+Ez9p6y9T8VBGIp3o1fAT9D7aNqmcIvOkWL2IvUDNM6dVWGBP1ZQ== X-Received: by 2002:a5d:6c6b:0:b0:1ea:77ea:dde8 with SMTP id r11-20020a5d6c6b000000b001ea77eadde8mr27428796wrz.690.1655801332854; Tue, 21 Jun 2022 01:48:52 -0700 (PDT) Received: from ?IPV6:2003:d8:2f04:2500:cdb0:9b78:d423:43f? (p200300d82f042500cdb09b78d423043f.dip0.t-ipconnect.de. [2003:d8:2f04:2500:cdb0:9b78:d423:43f]) by smtp.gmail.com with ESMTPSA id k7-20020a7bc407000000b0039c747a1e8fsm23276149wmi.7.2022.06.21.01.48.51 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 21 Jun 2022 01:48:52 -0700 (PDT) Message-ID: <506888c0-c257-e2a8-9540-823acdd403db@redhat.com> Date: Tue, 21 Jun 2022 10:48:51 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.9.0 To: Nadav Amit , linux-mm@kvack.org Cc: Nadav Amit , Mike Kravetz , Hugh Dickins , Andrew Morton , Axel Rasmussen , Peter Xu , Mike Rapoport References: <20220619233449.181323-1-namit@vmware.com> <20220619233449.181323-3-namit@vmware.com> From: David Hildenbrand Organization: Red Hat Subject: Re: [RFC PATCH v2 2/5] userfaultfd: introduce access-likely mode for copy/wp operations In-Reply-To: <20220619233449.181323-3-namit@vmware.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1655801338; a=rsa-sha256; cv=none; b=xuuSO8XJk7L2n1lBGtMw3eB6jcfxLr9B2tsAmALk/yLWT3wPblMHTl81DeK8pUvsDn/bOH 0Wy4mcjfLN65mx1SCogyX5StTPOe8YwOKukNIlieaI/fMmVR34DNWn2ayhussNDFFzay07 KIykKN+YmdspeZAx80hbGjzPXDMVn6c= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=NHNH4sqd; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf17.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=david@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1655801338; 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=w9DrM7fA/Vr1/hLg7C5EtTv+FJvTNPajneiswujjz00=; b=Kgh7E0C5dPR6vS0AXUC5e9jJlvNgpLfYkIbUJNj9rmG5udeqWU1LVAkVFilJpVp2jSV1s6 pnQ84BR22uNxT6WJjQUTujP9TJCWKHINIrZflaCMgHTioARrv9ztZQGPL92iaP3rl2BSvf UpF/IOKNFmjRSTRIwFqUiZI4kCee1b4= Authentication-Results: imf17.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=NHNH4sqd; dmarc=pass (policy=none) header.from=redhat.com; spf=none (imf17.hostedemail.com: domain of david@redhat.com has no SPF policy when checking 170.10.133.124) smtp.mailfrom=david@redhat.com X-Stat-Signature: jnki78zm7i6h3euedb3pygcbty9gbpek X-Rspamd-Queue-Id: A6FE7400AC X-Rspam-User: X-Rspamd-Server: rspam03 X-HE-Tag: 1655801338-818480 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 20.06.22 01:34, Nadav Amit wrote: > From: Nadav Amit > > Using a PTE on x86 with cleared access-bit (aka young-bit) > takes ~600 cycles more than when the access bit is set. At the same > time, setting the access-bit for memory that is not used (e.g., > prefetched) can introduce greater overheads, as the prefetched memory is > reclaimed later than it should be. > > Userfaultfd currently does not set the access-bit (excluding the > huge-pages case). Arguably, it is best to let the user control whether > the access bit should be set or not. The expected use is to request > userfaultfd to set the access-bit when the copy/wp operation is done to > resolve a page-fault, and not to set the access-bit when the memory is > prefetched. > > Introduce UFFDIO_COPY_MODE_ACCESS_LIKELY and > UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY to enable userspace to request > the young bit to be set. Set for UFFDIO_CONTINUE and UFFDIO_ZEROPAGE the > bit unconditionally since the former is only used to resolve page-faults > and the latter would not benefit from not setting the access-bit. > > Cc: Mike Kravetz > Cc: Hugh Dickins > Cc: Andrew Morton > Cc: Axel Rasmussen > Cc: Peter Xu > Cc: David Hildenbrand > Cc: Mike Rapoport > Signed-off-by: Nadav Amit > --- > fs/userfaultfd.c | 23 ++++++++++++++++------- > include/linux/userfaultfd_k.h | 1 + > include/uapi/linux/userfaultfd.h | 20 +++++++++++++++++++- > mm/userfaultfd.c | 18 ++++++++++++++++-- > 4 files changed, 52 insertions(+), 10 deletions(-) > > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 5daafa54eb3f..35a8c4347c54 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -1700,7 +1700,7 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, > struct uffdio_copy uffdio_copy; > struct uffdio_copy __user *user_uffdio_copy; > struct userfaultfd_wake_range range; > - bool mode_wp; > + bool mode_wp, mode_access_likely; > uffd_flags_t uffd_flags; > > user_uffdio_copy = (struct uffdio_copy __user *) arg; > @@ -1726,12 +1726,15 @@ static int userfaultfd_copy(struct userfaultfd_ctx *ctx, > ret = -EINVAL; > if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src) > goto out; > - if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP)) > + if (uffdio_copy.mode & ~(UFFDIO_COPY_MODE_DONTWAKE|UFFDIO_COPY_MODE_WP| > + UFFDIO_COPY_MODE_ACCESS_LIKELY)) > goto out; > > mode_wp = uffdio_copy.mode & UFFDIO_COPY_MODE_WP; > + mode_access_likely = uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY; I *relly* prefer just if (uffdio_copy.mode & UFFDIO_COPY_MODE_ACCESS_LIKELY) uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY [...] > > - uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0); > + uffd_flags = (mode_wp ? UFFD_FLAGS_WP : 0) | > + (mode_access_likely ? UFFD_FLAGS_ACCESS_LIKELY : 0); > Dito. > if (mmget_not_zero(ctx->mm)) { > ret = mwriteprotect_range(ctx->mm, uffdio_wp.range.start, > @@ -1871,6 +1877,7 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > struct uffdio_continue uffdio_continue; > struct uffdio_continue __user *user_uffdio_continue; > struct userfaultfd_wake_range range; > + uffd_flags_t uffd_flags; > > user_uffdio_continue = (struct uffdio_continue __user *)arg; > > @@ -1898,10 +1905,12 @@ static int userfaultfd_continue(struct userfaultfd_ctx *ctx, unsigned long arg) > if (uffdio_continue.mode & ~UFFDIO_CONTINUE_MODE_DONTWAKE) > goto out; > > + uffd_flags = UFFD_FLAGS_ACCESS_LIKELY; Can we add a comment why that makes sense? I think I know why -- someone is stuck waiting for that continue to happen :) > + > if (mmget_not_zero(ctx->mm)) { > ret = mcopy_continue(ctx->mm, uffdio_continue.range.start, > uffdio_continue.range.len, > - &ctx->mmap_changing, 0); > + &ctx->mmap_changing, uffd_flags); > mmput(ctx->mm); > } else { > return -ESRCH; > diff --git a/include/linux/userfaultfd_k.h b/include/linux/userfaultfd_k.h > index 6331148023c1..e6ac165ec044 100644 > --- a/include/linux/userfaultfd_k.h > +++ b/include/linux/userfaultfd_k.h > @@ -58,6 +58,7 @@ enum mcopy_atomic_mode { > typedef unsigned int __bitwise uffd_flags_t; > > #define UFFD_FLAGS_WP ((__force uffd_flags_t)BIT(0)) > +#define UFFD_FLAGS_ACCESS_LIKELY ((__force uffd_flags_t)BIT(1)) > > extern int mfill_atomic_install_pte(struct mm_struct *dst_mm, pmd_t *dst_pmd, > struct vm_area_struct *dst_vma, > diff --git a/include/uapi/linux/userfaultfd.h b/include/uapi/linux/userfaultfd.h > index 005e5e306266..d9c8ce9ba777 100644 > --- a/include/uapi/linux/userfaultfd.h > +++ b/include/uapi/linux/userfaultfd.h > @@ -38,7 +38,8 @@ > UFFD_FEATURE_MINOR_HUGETLBFS | \ > UFFD_FEATURE_MINOR_SHMEM | \ > UFFD_FEATURE_EXACT_ADDRESS | \ > - UFFD_FEATURE_WP_HUGETLBFS_SHMEM) > + UFFD_FEATURE_WP_HUGETLBFS_SHMEM | \ > + UFFD_FEATURE_ACCESS_HINTS) > #define UFFD_API_IOCTLS \ > ((__u64)1 << _UFFDIO_REGISTER | \ > (__u64)1 << _UFFDIO_UNREGISTER | \ > @@ -203,6 +204,10 @@ struct uffdio_api { > * > * UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd > * write-protection mode is supported on both shmem and hugetlbfs. > + * > + * UFFD_FEATURE_ACCESS_HINTS indicates that the copy supports > + * UFFDIO_COPY_MODE_ACCESS_LIKELY supports > + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY. I think that sentence doesn't make sense. > */ > #define UFFD_FEATURE_PAGEFAULT_FLAG_WP (1<<0) > #define UFFD_FEATURE_EVENT_FORK (1<<1) > @@ -217,6 +222,7 @@ struct uffdio_api { > #define UFFD_FEATURE_MINOR_SHMEM (1<<10) > #define UFFD_FEATURE_EXACT_ADDRESS (1<<11) > #define UFFD_FEATURE_WP_HUGETLBFS_SHMEM (1<<12) > +#define UFFD_FEATURE_ACCESS_HINTS (1<<13) > __u64 features; > > __u64 ioctls; > @@ -260,6 +266,13 @@ struct uffdio_copy { > * copy_from_user will not read the last 8 bytes. > */ > __s64 copy; > + /* > + * UFFDIO_COPY_MODE_ACCESS_LIKELY will set the mapped page as young. Setting the page young is an implementation detail. Can you phrase it more generically what the effect of that hint might be? > + * This can reduce the time that the first access to the page takes. > + * Yet, if set opportunistically to memory that is not used, it might > + * extend the time before the unused memory pages are reclaimed. > + */ > +#define UFFDIO_COPY_MODE_ACCESS_LIKELY ((__u64)1<<3) > }; > > struct uffdio_zeropage { > @@ -284,6 +297,10 @@ struct uffdio_writeprotect { > * UFFDIO_WRITEPROTECT_MODE_DONTWAKE: set the flag to avoid waking up > * any wait thread after the operation succeeds. > * > + * UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY: set the flag to mark the modified > + * memory as young, which can reduce the time that the first access > + * to the page takes. Dito. > + * > * NOTE: Write protecting a region (WP=1) is unrelated to page faults, > * therefore DONTWAKE flag is meaningless with WP=1. Removing write > * protection (WP=0) in response to a page fault wakes the faulting > @@ -291,6 +308,7 @@ struct uffdio_writeprotect { > */ > #define UFFDIO_WRITEPROTECT_MODE_WP ((__u64)1<<0) > #define UFFDIO_WRITEPROTECT_MODE_DONTWAKE ((__u64)1<<1) > +#define UFFDIO_WRITEPROTECT_MODE_ACCESS_LIKELY ((__u64)1<<2) > __u64 mode; > }; [...] > @@ -691,6 +699,9 @@ ssize_t mfill_zeropage(struct mm_struct *dst_mm, unsigned long start, > unsigned long len, atomic_t *mmap_changing, > uffd_flags_t uffd_flags) > { > + /* There is no cost for setting the access bit of a zeropage */ > + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; > + > return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_ZEROPAGE, > mmap_changing, 0); > } > @@ -699,6 +710,9 @@ ssize_t mcopy_continue(struct mm_struct *dst_mm, unsigned long start, > unsigned long len, atomic_t *mmap_changing, > uffd_flags_t uffd_flags) > { > + /* The page is likely to be accessed */ > + uffd_flags |= UFFD_FLAGS_ACCESS_LIKELY; Shoouldn't that be set by the caller already? > + > return __mcopy_atomic(dst_mm, start, 0, len, MCOPY_ATOMIC_CONTINUE, > mmap_changing, 0); > } In general, LGTM. -- Thanks, David / dhildenb