From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 00E41313546 for ; Mon, 2 Feb 2026 21:49:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=170.10.129.124 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770068958; cv=none; b=H/Rl6bSfpMejUC0KsCitPR5+RHTOyx+gf73wa6HarvmUGy8HO38Lj+Mq+FGedGdPzl/osq7GAwPNIXv/+gZBA7fuUY90668ImJcnQDMvj11oeDIoG08mWPLRkcFq6RFPxAxkwXdSMWiTPtAuzSQUw4q8+MCa6ZPAUGwpDcb5Qxo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770068958; c=relaxed/simple; bh=agtCQ9pZwtAROLqQxsM2KH3S9+KhShyu1IMY1wa4cPE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KMgf+mURvuLaqHpNeF4oYMAWrmhv3sWCMnbJOP4EGZAQDQE652ZXXnajGCRS3cvSKE93VcKA8TO+M9zOVspW5mZ5rT4tugeUr7zhZG6mZfFXYbE0M7qtNukR8HTJBRrs3LJlaJOBZLpAKFPqXbG0vFVQWHB+0Es3fuMHDMyZsqc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com; spf=pass smtp.mailfrom=redhat.com; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b=hfF1yLKY; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b=CWHJ+xyU; arc=none smtp.client-ip=170.10.129.124 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=redhat.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=redhat.com header.i=@redhat.com header.b="hfF1yLKY"; dkim=pass (2048-bit key) header.d=redhat.com header.i=@redhat.com header.b="CWHJ+xyU" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770068954; 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: in-reply-to:in-reply-to:references:references; bh=g5FHEvnvdTlqMH7yVdF7d/M75bDKDwBnGXhGmTp6Y0Q=; b=hfF1yLKYtGRLJMUp1DwTJd9AsMilJ2Yrk6950HmA4JPLoXNFjSHpPQEKm6TviBv0EuT5oq iIbzZE+ihW+rxXUZe5/SxbppPCtwK6cILoGJ4TnimgF7URsxGrGuYPZz7wl9Q3v/UtYfHg inYuunZlDk8GBaPl+btAiMXI6X0Dc50= Received: from mail-qt1-f198.google.com (mail-qt1-f198.google.com [209.85.160.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-297-AcnW_GgiPvy9W2GLD_-Htg-1; Mon, 02 Feb 2026 16:49:13 -0500 X-MC-Unique: AcnW_GgiPvy9W2GLD_-Htg-1 X-Mimecast-MFC-AGG-ID: AcnW_GgiPvy9W2GLD_-Htg_1770068952 Received: by mail-qt1-f198.google.com with SMTP id d75a77b69052e-5019f8a18cdso164951721cf.2 for ; Mon, 02 Feb 2026 13:49:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=google; t=1770068952; x=1770673752; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=g5FHEvnvdTlqMH7yVdF7d/M75bDKDwBnGXhGmTp6Y0Q=; b=CWHJ+xyUBII2jBt4GavcJzZrJIIbufyeeNNKjI8Iiw+kDqbP4nChIVv6IezF20iHjp 7TudGIn91aoOBqXB6p5cWER5hWGrjbIVupk2DPAy0Pkqh5yk7TR2a4BiCPO+3ZHTnCOJ lKRgPsJ7/Wdj41ceyvdjK7TCTiWdgK2v1oavzcCc925kfYC47hwfRC7qOgyty0nQRqAs jZN4j5+pNh7pbwSvLQ0EphOkdcOoaAx0rYRI+3l1vn1ViNXyTW5mSV2s2msfFr95MIs8 Hv9/lGfI/mNxlF9e6spfO9dl3vE2vU2dUDaIHWhz7mXoWsjlYiavYTQExpFKcHyVp0Zg +vjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770068952; x=1770673752; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=g5FHEvnvdTlqMH7yVdF7d/M75bDKDwBnGXhGmTp6Y0Q=; b=jwP8tT7XkZKWVVjnxdrTgDHrpGjyzLMn3M2KgiDwuR0Ng3iP4a5lFPIcfADcXeZAAZ 7BVZu+pkAkJ1dan0ONtspkC26ZKHecE4cYwgSAc5tCFx2WemvNWhgCth/vGwGrZhtwiN zXKhqy5Feq1duge1+uLji2qejXOHTEYwpZY/CaTr1RHlBOZQgxi2TIBSY0Ld0A2r3zBm 0xS80J0Dcs2P+wFZpNtwZ0Y0Gqku3yCf07qfaqV62pnduil5n4xrKBPOnxGr6QRydtWI O5MdP0cbPZB3HQ5dZBY1jNKn6CEtfNsqlCg6DuKl8ph6P06QlHE6U6KJhCl041B/dw3Y jsyw== X-Forwarded-Encrypted: i=1; AJvYcCUuFxPINyH5A3KRCHEEet6H7vYDnxDAh2bGqU9gaVyOJyDoM19ab5dsQHLdkVmZoJtGhpy0l4I8DYpsxu/LBaY=@vger.kernel.org X-Gm-Message-State: AOJu0Yz3XWdaTrU53WfshogoAEgwMLAyBk47Cys2PC0kNuDqLcT92KSo nHXlCAWIqKuFAUVbCKnnZl53TtFtXSOkA9l3Y3xrDaySKh1wwMKjUznJ4Dh2/TUKcVwvyHrC2jv vyrfuWWRUBQG8FSfsptJKakfpr5xo5IFOm+6CFHsLZdw5gtCmOpkBCueucoFMqn/565vr4g== X-Gm-Gg: AZuq6aL2bjMWdJZcoU35wvel3hx30RaCkrk/V5tuH103cO6VONPcgSotyaIWg99K/aB GageKA82riP2JJS6FcqnM6X1F0nKX0zJvlX0Pqj1yW6Jji62tcvvwpRNqP1VJofhbugdalGq4pQ LO2l+7IanEJJx89SERjgrhaMdP1fScZ0/8bN61UWfb9WII5ft5b2PyyFY+0JYrGeh8zBMN2ciC8 l1fcoCI6qdSfkXFt/MiwAdrJl8kAvRLq0xFgxZ4xrLPA8DwsiQDUMWuVzBo8vGkIMnuCinmKUD5 42A5gJnEshw5t844PCujspomJmVaXZz8/L9tD2e0SWlor/QYlZ4SjjutEu1MTfIQwFuw5XZIuUB fotA= X-Received: by 2002:ac8:5a92:0:b0:502:aff1:5689 with SMTP id d75a77b69052e-505d2152eb7mr156388541cf.7.1770068952351; Mon, 02 Feb 2026 13:49:12 -0800 (PST) X-Received: by 2002:ac8:5a92:0:b0:502:aff1:5689 with SMTP id d75a77b69052e-505d2152eb7mr156388151cf.7.1770068951834; Mon, 02 Feb 2026 13:49:11 -0800 (PST) Received: from x1.local ([142.188.210.156]) by smtp.gmail.com with ESMTPSA id d75a77b69052e-50337bbbd2csm114145581cf.25.2026.02.02.13.49.10 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 02 Feb 2026 13:49:11 -0800 (PST) Date: Mon, 2 Feb 2026 16:49:09 -0500 From: Peter Xu To: Mike Rapoport Cc: linux-mm@kvack.org, Andrea Arcangeli , Andrew Morton , Axel Rasmussen , Baolin Wang , David Hildenbrand , Hugh Dickins , James Houghton , "Liam R. Howlett" , Lorenzo Stoakes , Michal Hocko , Muchun Song , Nikita Kalyazin , Oscar Salvador , Paolo Bonzini , Sean Christopherson , Shuah Khan , Suren Baghdasaryan , Vlastimil Babka , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH RFC 04/17] userfaultfd: introduce mfill_get_vma() and mfill_put_vma() Message-ID: References: <20260127192936.1250096-1-rppt@kernel.org> <20260127192936.1250096-5-rppt@kernel.org> Precedence: bulk X-Mailing-List: linux-kselftest@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20260127192936.1250096-5-rppt@kernel.org> Hi, Mike, On Tue, Jan 27, 2026 at 09:29:23PM +0200, Mike Rapoport wrote: > From: "Mike Rapoport (Microsoft)" > > Split the code that finds, locks and verifies VMA from mfill_atomic() > into a helper function. > > This function will be used later during refactoring of > mfill_atomic_pte_copy(). > > Add a counterpart mfill_put_vma() helper that unlocks the VMA and > releases map_changing_lock. > > Signed-off-by: Mike Rapoport (Microsoft) > --- > mm/userfaultfd.c | 124 ++++++++++++++++++++++++++++------------------- > 1 file changed, 73 insertions(+), 51 deletions(-) > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > index 9dd285b13f3b..45d8f04aaf4f 100644 > --- a/mm/userfaultfd.c > +++ b/mm/userfaultfd.c > @@ -157,6 +157,73 @@ static void uffd_mfill_unlock(struct vm_area_struct *vma) > } > #endif > > +static void mfill_put_vma(struct mfill_state *state) > +{ > + up_read(&state->ctx->map_changing_lock); > + uffd_mfill_unlock(state->vma); > + state->vma = NULL; > +} > + > +static int mfill_get_vma(struct mfill_state *state) > +{ > + struct userfaultfd_ctx *ctx = state->ctx; > + uffd_flags_t flags = state->flags; > + struct vm_area_struct *dst_vma; > + int err; > + > + /* > + * Make sure the vma is not shared, that the dst range is > + * both valid and fully within a single existing vma. > + */ > + dst_vma = uffd_mfill_lock(ctx->mm, state->dst_start, state->len); > + if (IS_ERR(dst_vma)) > + return PTR_ERR(dst_vma); > + > + /* > + * If memory mappings are changing because of non-cooperative > + * operation (e.g. mremap) running in parallel, bail out and > + * request the user to retry later > + */ > + down_read(&ctx->map_changing_lock); > + err = -EAGAIN; > + if (atomic_read(&ctx->mmap_changing)) > + goto out_unlock; > + > + err = -EINVAL; > + > + /* > + * shmem_zero_setup is invoked in mmap for MAP_ANONYMOUS|MAP_SHARED but > + * it will overwrite vm_ops, so vma_is_anonymous must return false. > + */ > + if (WARN_ON_ONCE(vma_is_anonymous(dst_vma) && > + dst_vma->vm_flags & VM_SHARED)) > + goto out_unlock; > + > + /* > + * validate 'mode' now that we know the dst_vma: don't allow > + * a wrprotect copy if the userfaultfd didn't register as WP. > + */ > + if ((flags & MFILL_ATOMIC_WP) && !(dst_vma->vm_flags & VM_UFFD_WP)) > + goto out_unlock; > + > + if (is_vm_hugetlb_page(dst_vma)) > + goto out; > + > + if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma)) > + goto out_unlock; > + if (!vma_is_shmem(dst_vma) && > + uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE)) > + goto out_unlock; IMHO it's a bit weird to check for vma permissions in a get_vma() function. Also, in the follow up patch it'll be also reused in mfill_copy_folio_retry() which doesn't need to check vma permission. Maybe we can introduce mfill_vma_check() for these two checks? Then we can also drop the slightly weird is_vm_hugetlb_page() check (and "out" label) above. > + > +out: > + state->vma = dst_vma; > + return 0; > + > +out_unlock: > + mfill_put_vma(state); > + return err; > +} > + > static pmd_t *mm_alloc_pmd(struct mm_struct *mm, unsigned long address) > { > pgd_t *pgd; > @@ -768,8 +835,6 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx, > .src_addr = src_start, > .dst_addr = dst_start, > }; > - struct mm_struct *dst_mm = ctx->mm; > - struct vm_area_struct *dst_vma; > long copied = 0; > ssize_t err; > > @@ -784,57 +849,17 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx, > VM_WARN_ON_ONCE(dst_start + len <= dst_start); > > retry: > - /* > - * Make sure the vma is not shared, that the dst range is > - * both valid and fully within a single existing vma. > - */ > - dst_vma = uffd_mfill_lock(dst_mm, dst_start, len); > - if (IS_ERR(dst_vma)) { > - err = PTR_ERR(dst_vma); > + err = mfill_get_vma(&state); > + if (err) > goto out; > - } > - > - /* > - * If memory mappings are changing because of non-cooperative > - * operation (e.g. mremap) running in parallel, bail out and > - * request the user to retry later > - */ > - down_read(&ctx->map_changing_lock); > - err = -EAGAIN; > - if (atomic_read(&ctx->mmap_changing)) > - goto out_unlock; > - > - err = -EINVAL; > - /* > - * shmem_zero_setup is invoked in mmap for MAP_ANONYMOUS|MAP_SHARED but > - * it will overwrite vm_ops, so vma_is_anonymous must return false. > - */ > - if (WARN_ON_ONCE(vma_is_anonymous(dst_vma) && > - dst_vma->vm_flags & VM_SHARED)) > - goto out_unlock; > - > - /* > - * validate 'mode' now that we know the dst_vma: don't allow > - * a wrprotect copy if the userfaultfd didn't register as WP. > - */ > - if ((flags & MFILL_ATOMIC_WP) && !(dst_vma->vm_flags & VM_UFFD_WP)) > - goto out_unlock; > > /* > * If this is a HUGETLB vma, pass off to appropriate routine > */ > - if (is_vm_hugetlb_page(dst_vma)) > - return mfill_atomic_hugetlb(ctx, dst_vma, dst_start, > + if (is_vm_hugetlb_page(state.vma)) > + return mfill_atomic_hugetlb(ctx, state.vma, dst_start, > src_start, len, flags); > > - if (!vma_is_anonymous(dst_vma) && !vma_is_shmem(dst_vma)) > - goto out_unlock; > - if (!vma_is_shmem(dst_vma) && > - uffd_flags_mode_is(flags, MFILL_ATOMIC_CONTINUE)) > - goto out_unlock; > - > - state.vma = dst_vma; > - > while (state.src_addr < src_start + len) { > VM_WARN_ON_ONCE(state.dst_addr >= dst_start + len); > > @@ -853,8 +878,7 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx, > if (unlikely(err == -ENOENT)) { > void *kaddr; > > - up_read(&ctx->map_changing_lock); > - uffd_mfill_unlock(state.vma); > + mfill_put_vma(&state); > VM_WARN_ON_ONCE(!state.folio); > > kaddr = kmap_local_folio(state.folio, 0); > @@ -883,9 +907,7 @@ static __always_inline ssize_t mfill_atomic(struct userfaultfd_ctx *ctx, > break; > } > > -out_unlock: > - up_read(&ctx->map_changing_lock); > - uffd_mfill_unlock(state.vma); > + mfill_put_vma(&state); > out: > if (state.folio) > folio_put(state.folio); > -- > 2.51.0 > -- Peter Xu