From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (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 6A7A2394498 for ; Wed, 5 Aug 2026 11:00:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785927656; cv=none; b=fSg+sF2MrydZ+hW8zaSr98MMEKSufHEMu7TWHFBEQs5CANhFHkANdCbCcLnKl23vAWaQYaKn27yBFb3YqQNxPGGE7UHQqqnmHa/JqQmvgeJ/8ufp7lOLlW4l6fAjRTOoSp4sZaIZ5J6aIIZb0JwkCv5A6aHZQJGByc0aVEntVq4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1785927656; c=relaxed/simple; bh=dnj5bGbdkD03D33Pta8H2E7/kbYp4LcxAu37C9HJ3+4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=izRe7byM80z4HvOOrHhJhsaUQOva34EPGFPlfyyOtzCQ4MHc95j2aRisGJKKw/VzWFHqCvvfYpXpX3gIP9L1/N21Y0Rd0BZt/GxWdQYcfT+DyEebDGqe3vYQ1XnhKUC/x3oOsoQrp35Iw1YcmFRX3uBIwHn9T4Mx9hOZqC222iE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=KSGDHbe/; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="KSGDHbe/" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 8E58A1F000E9; Wed, 5 Aug 2026 11:00:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1785927655; bh=qDsAuiAwi/V/y/EO85Gmfy8jVW1Pmelx7+mpYdcm6Zw=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=KSGDHbe/7YJtFj0m6LTmJpBaFhVs4pI+dlDOkzJJneOzSG/8GNA3lzF5zTblku03H Dc8YqSOTy6EhJRhp68QHmNeFaP+VeFaX0Ayp4VwHgcphj2jokMinhNCLpHuT6vQFmb r0CP0o00IkvU2vGB1d55NZbokuqhC29gaDmC0Xmu+4BMeDR5d6VodStxZfD4+eaLfj 8IQGlJH+BQ1FQ5p8Vj/vZOjthOW/4KgTYjMXi784ImfMxxitGx7sqvVsSA0lrEVmNW f6d1jt9PLXumf8BQLJaKSHMj5q+Ynd1+8t9hJ5WWIQjdDRRgLjmu58OS/Icodq/OEz MCt4hEYwpDQZA== Date: Wed, 5 Aug 2026 12:00:37 +0100 From: "Lorenzo Stoakes (ARM)" To: Barry Song Cc: Hongru Zhang , akpm@linux-foundation.org, linux-mm@kvack.org, david@kernel.org, liam@infradead.org, linux-kernel@vger.kernel.org, mhocko@suse.com, rppt@kernel.org, shakeel.butt@linux.dev, surenb@google.com, vbabka@kernel.org, willy@infradead.org, zhanghongru@xiaomi.com Subject: Re: [RFC PATCH v4 1/3] mm: allow page faults to request VMA-lock retry Message-ID: References: <20260804095219.46213-1-zhanghongru@xiaomi.com> 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=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Wed, Aug 05, 2026 at 05:13:49AM +0800, Barry Song wrote: > On Tue, Aug 4, 2026 at 8:32 PM Lorenzo Stoakes (ARM) wrote: > > > > On Tue, Aug 04, 2026 at 05:52:19PM +0800, Hongru Zhang wrote: > > > From: Hongru Zhang > > > > > > Page faults handled under the per-VMA lock currently fall back to the > > > mmap_lock path whenever handle_mm_fault() returns VM_FAULT_RETRY. This > > > means that lower-level fault handlers have no way to tell the > > > architecture fault handler that the retry can safely continue under the > > > per-VMA lock. > > > > > > Add VM_FAULT_MAY_USE_VMA_LOCK as an advisory bit that can be returned > > > > I don't love that name or that faulting retry behaviour is _modified_ by a > > value that indicates fault resolution state... ugh. > > > > It's kinda confusing things 'VM_FAULT_RETRY' is 'you have to retry this > > fault'. > > > > 'VM_FAULT_MAY_...' is starting to bring in effectively configuration > > options into it and that's kinda horrible. > > > > I mean is there any reason we shouldn't ALWAYS do this if a VMA lock was > > used? > > > > It's not too expensive to do a single retry with the VMA lock before > > falling back to the mmap lock. > > > > So maybe simplify like that? > > > > And like that this series becomes a single patch right? > > This is a brilliant idea. That's a genius insight, Lorenzo. Haha thanks! :) > > I guess the conceptual model could simply be: > > diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c > index 45b99c3b1442..3592bcc9bbd7 100644 > --- a/arch/x86/mm/fault.c > +++ b/arch/x86/mm/fault.c > @@ -1222,6 +1222,7 @@ void do_user_addr_fault(struct pt_regs *regs, > struct mm_struct *mm; > vm_fault_t fault; > unsigned int flags = FAULT_FLAG_DEFAULT; > + bool vma_lock_retried = false; > > tsk = current; > mm = tsk->mm; > @@ -1331,6 +1332,7 @@ void do_user_addr_fault(struct pt_regs *regs, > if (!(flags & FAULT_FLAG_USER)) > goto lock_mmap; > > +vma_lock: > vma = lock_vma_under_rcu(mm, address); > if (!vma) > goto lock_mmap; > @@ -1352,6 +1354,11 @@ void do_user_addr_fault(struct pt_regs *regs, > if (fault & VM_FAULT_MAJOR) > flags |= FAULT_FLAG_TRIED; > > + if (!vma_lock_retried) { > + vma_lock_retried = true; > + goto vma_lock; > + } > + > /* Quick path to respond to signals */ > if (fault_signal_pending(fault, regs)) { > if (!user_mode(regs)) > I seem to remember Willy didn't love the idea of '1 more try with the VMA lock' but this isn't _quite_ doing that. If we spuriously can't get the VMA lock then this gives up immediately and goes to the mmap logic without a retry, so we're not doing that on lock contention at least. (We could fix that with vma_start_read_unlocked() though which would handle write lock contention by sleeping on mmap read lock until the VMA lock can be obtained - though we have to be careful about possible lock inversion vs. a writer maybe?). So it only retries quickly if a retry is requested by the fault logic. I guess it does end up working nicely then - because if the retry can immediately succeed with a VMA lock again then it does that, but if it can't then it falls through to the mmap lock quickly. (And use of vma_start_read_unlocked() would make that more reliable vs. lock contention.) I think there were cases where we thought that might be the case (though it then makes you wonder why exactly the fault needs a retry?) (This is assuming nothing in the fault path would sleep holding the VMA lock, which I don't think can happen?). > Nothing else needs to change then. I wonder if there is a cleaner > way to implement the idea, but it is really stunning. Thanks again :>) I'm not quite sure this is really all that clever, but that's nice of you :) > > Best Regards > Barry -- Cheers, Lorenzo