From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (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 DA310487BE; Sun, 8 Feb 2026 09:49:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770544178; cv=none; b=RY2HdYKEFo8BFOLVgRBj0Ssv6ibPCUTQBemkDOJRrcyIcETgyakpvibP42JZK3vCy4lpqu1oIjvj7Wuwp9gho76f9Qw99b392s6T0lddC7F5F1lvPZlWWsrBADs2+WWoV768GvtHPTRJfJ1vnv8YenyY8ujOXfon5xVQ0tdUCHM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770544178; c=relaxed/simple; bh=dad1qQG14Id6LY4XasdkZA/u/K3SW5eX6EZX2zBf4BM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=h4bAAJVvfaH5hCqiaWrzBgVe1UbW6ouJcy1wtO3wu/JlUWYX/7GszRhXnculg34gvq5GBB6FaII5DZ/rEkietZcdxiyS6SPvzfT8jEP+BMD6ndggZyEZeDvvSoMdNMHbi6XCukbXkvl6zd6ZCehkBc9NmwTdcSytND7aiXHUc1w= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=Hz7CIBWk; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="Hz7CIBWk" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 649B7C4CEF7; Sun, 8 Feb 2026 09:49:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1770544178; bh=dad1qQG14Id6LY4XasdkZA/u/K3SW5eX6EZX2zBf4BM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Hz7CIBWk+BanWnWsjwmGQuuBnJ6GS1FGyOLDJGQ6voZfjDwkbQPPVzj7mof+SYYku aZJZQWvuySWH9WBmsc5GVAry8qs4nFCtuWtttSVxzxoPDgpwjzYgg+xobfkrpqARR6 yhmN2j6+GacCPdoL8CnXX3l/J6OuHmbMtnDHR0LDesuHToOppkkaIoyoTTGoLA6SHw Se/YmgMtvCoJHuiNd3WkaRPy5yyoNdl18MPxXLjXnjaiDLgepDWeYkwJmR5Bf5gNzk 6feyIqqMfoHSexciIygXgmQrXIAonUw05Nko9juey1Bh8uOoF6h/C1tlSGlXY+lFpb wevcUFtyXv0Bw== Date: Sun, 8 Feb 2026 11:49:27 +0200 From: Mike Rapoport To: Peter Xu 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 01/17] userfaultfd: introduce mfill_copy_folio_locked() helper Message-ID: References: <20260127192936.1250096-1-rppt@kernel.org> <20260127192936.1250096-2-rppt@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Hi Peter, On Tue, Feb 03, 2026 at 12:45:02PM -0500, Peter Xu wrote: > On Tue, Jan 27, 2026 at 09:29:20PM +0200, Mike Rapoport wrote: > > From: "Mike Rapoport (Microsoft)" > > > > Split copying of data when locks held from mfill_atomic_pte_copy() into > > a helper function mfill_copy_folio_locked(). > > > > This makes improves code readability and makes complex > > mfill_atomic_pte_copy() function easier to comprehend. > > > > No functional change. > > > > Signed-off-by: Mike Rapoport (Microsoft) > > The movement looks all fine, > > Acked-by: Peter Xu Thanks! > Just one pure question to ask. > > > --- > > mm/userfaultfd.c | 59 ++++++++++++++++++++++++++++-------------------- > > 1 file changed, 35 insertions(+), 24 deletions(-) > > > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c > > index e6dfd5f28acd..a0885d543f22 100644 > > --- a/mm/userfaultfd.c > > +++ b/mm/userfaultfd.c > > @@ -238,6 +238,40 @@ int mfill_atomic_install_pte(pmd_t *dst_pmd, > > return ret; > > } > > > > +static int mfill_copy_folio_locked(struct folio *folio, unsigned long src_addr) > > +{ > > + void *kaddr; > > + int ret; > > + > > + kaddr = kmap_local_folio(folio, 0); > > + /* > > + * The read mmap_lock is held here. Despite the > > + * mmap_lock being read recursive a deadlock is still > > + * possible if a writer has taken a lock. For example: > > + * > > + * process A thread 1 takes read lock on own mmap_lock > > + * process A thread 2 calls mmap, blocks taking write lock > > + * process B thread 1 takes page fault, read lock on own mmap lock > > + * process B thread 2 calls mmap, blocks taking write lock > > + * process A thread 1 blocks taking read lock on process B > > + * process B thread 1 blocks taking read lock on process A > > While moving, I wonder if we need this complex use case to describe the > deadlock. Shouldn't this already happen with 1 process only? > > process A thread 1 takes read lock (e.g. reaching here but > before copy_from_user) > process A thread 2 calls mmap, blocks taking write lock > process A thread 1 goes on copy_from_user(), trigger page fault, > then tries to re-take the read lock > > IIUC above should already cause deadlock when rwsem prioritize the write > lock here. We surely can improve the description here, but it should be a separate patch with its own changelog and it's out of scope of this series. > > + * > > + * Disable page faults to prevent potential deadlock > > + * and retry the copy outside the mmap_lock. > > + */ > > + pagefault_disable(); > > + ret = copy_from_user(kaddr, (const void __user *) src_addr, > > + PAGE_SIZE); > > + pagefault_enable(); > > + kunmap_local(kaddr); -- Sincerely yours, Mike.