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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5A7DBC7EE2F for ; Mon, 12 Jun 2023 18:45:03 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235943AbjFLSpB (ORCPT ); Mon, 12 Jun 2023 14:45:01 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59398 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237291AbjFLSot (ORCPT ); Mon, 12 Jun 2023 14:44:49 -0400 Received: from mail-oo1-xc2c.google.com (mail-oo1-xc2c.google.com [IPv6:2607:f8b0:4864:20::c2c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 82C9C10F3 for ; Mon, 12 Jun 2023 11:44:45 -0700 (PDT) Received: by mail-oo1-xc2c.google.com with SMTP id 006d021491bc7-558b6cffe03so2692851eaf.3 for ; Mon, 12 Jun 2023 11:44:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1686595485; x=1689187485; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=gLSvKncaUgXX5sMVjgHiwvAyzgL2miMkV5TwRmL7eWs=; b=5Y4Vy2iismoAzZRhbru/V2dPtaEpSrjSWGAfWTaV/GCVgdTzE3uEYmWLtkAw83XnbU +w8KNafozBU6IE2VJ2xTusRtqZEYg5sPtWP9cRbzBxArC7cGXYVRD6XYTGQ6s5sEBHCw +sUusni7pX503tgT/34wEX1EtJtAGRxpI/m2DnVA/nXxgYJNaUWbmM0eDG2An8aq70td Kz6eDZ2DGLuBVm5y0DeUHhHJHAUU5Z8J1gZByNQ3zDilNMNn1asZLjd4UWbON/IxayMv z8BH/H+TXzGcm95HEHXW5Bs7LmwIOUCJRvd64FNrbT3Snso8giytf+Fs0svKVrb7Iq7+ GSaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1686595485; x=1689187485; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=gLSvKncaUgXX5sMVjgHiwvAyzgL2miMkV5TwRmL7eWs=; b=UYqc8YY8KAKp2jGd5bBG0TdSb4t5951qAS+bqJOnlBs6z4nDR2ajA+l5fPaH7rGmtS 9c72ElenYOfLUl3d6X66kLb2JyGwaCDCFIit9u2kTTH4eT/1SCOPvfaIJb+5NMj8nSDq PB1Yw63YSIk6PKd/HraKJ7XnJcgFeCjj4Ni4SwIMLzcOajKGC0i30GY53q3kaJ6k+Vmw S5h2BkeiF1TR06G+fNJlxyA/KfbTZSUlVEVJdHi4aRbzHrSEWlL/H1wfOfEiSKqkfaWu AjI/Yxcz4AR9d8krhAvcNAbjnpKkH0wEiIABcLkZk+QTnIX/s6hVl1VY4gFXWCSuzJr5 /4AA== X-Gm-Message-State: AC+VfDw5COZCA13HXS0i5NK6xGi10b0maKxbjjWlVRqU1LS8WKa+unVR K6KmJzTd7UKrbhTu3QjuZXSMN8KCbGpZU5PZLaOT2A== X-Google-Smtp-Source: ACHHUZ7GdKM3fdCF0BbRZ7LDjfdKukE+hxCwPT2UJ9z/E0fZdGEJitZ6xGzwak0ybXCSfMcrn6FPBI3A9bGC7wYAIZc= X-Received: by 2002:a05:6358:c603:b0:12b:e390:2b5f with SMTP id fd3-20020a056358c60300b0012be3902b5fmr1677492rwb.6.1686595484442; Mon, 12 Jun 2023 11:44:44 -0700 (PDT) MIME-Version: 1.0 References: <20230609005158.2421285-1-surenb@google.com> <20230609005158.2421285-5-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Mon, 12 Jun 2023 11:44:33 -0700 Message-ID: Subject: Re: [PATCH v2 4/6] mm: drop VMA lock before waiting for migration To: Peter Xu Cc: akpm@linux-foundation.org, willy@infradead.org, hannes@cmpxchg.org, mhocko@suse.com, josef@toxicpanda.com, jack@suse.cz, ldufour@linux.ibm.com, laurent.dufour@fr.ibm.com, michel@lespinasse.org, liam.howlett@oracle.com, jglisse@google.com, vbabka@suse.cz, minchan@google.com, dave@stgolabs.net, punit.agrawal@bytedance.com, lstoakes@gmail.com, hdanton@sina.com, apopple@nvidia.com, ying.huang@intel.com, david@redhat.com, yuzhao@google.com, dhowells@redhat.com, hughd@google.com, viro@zeniv.linux.org.uk, brauner@kernel.org, pasha.tatashin@soleen.com, linux-mm@kvack.org, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-team@android.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 12, 2023 at 11:34=E2=80=AFAM Peter Xu wrote= : > > On Mon, Jun 12, 2023 at 09:07:38AM -0700, Suren Baghdasaryan wrote: > > On Mon, Jun 12, 2023 at 6:36=E2=80=AFAM Peter Xu wr= ote: > > > > > > On Fri, Jun 09, 2023 at 06:29:43PM -0700, Suren Baghdasaryan wrote: > > > > On Fri, Jun 9, 2023 at 3:30=E2=80=AFPM Suren Baghdasaryan wrote: > > > > > > > > > > On Fri, Jun 9, 2023 at 1:42=E2=80=AFPM Peter Xu wrote: > > > > > > > > > > > > On Thu, Jun 08, 2023 at 05:51:56PM -0700, Suren Baghdasaryan wr= ote: > > > > > > > migration_entry_wait does not need VMA lock, therefore it can= be dropped > > > > > > > before waiting. Introduce VM_FAULT_VMA_UNLOCKED to indicate t= hat VMA > > > > > > > lock was dropped while in handle_mm_fault(). > > > > > > > Note that once VMA lock is dropped, the VMA reference can't b= e used as > > > > > > > there are no guarantees it was not freed. > > > > > > > > > > > > Then vma lock behaves differently from mmap read lock, am I rig= ht? Can we > > > > > > still make them match on behaviors, or there's reason not to do= so? > > > > > > > > > > I think we could match their behavior by also dropping mmap_lock = here > > > > > when fault is handled under mmap_lock (!(fault->flags & > > > > > FAULT_FLAG_VMA_LOCK)). > > > > > I missed the fact that VM_FAULT_COMPLETED can be used to skip dro= pping > > > > > mmap_lock in do_page_fault(), so indeed, I might be able to use > > > > > VM_FAULT_COMPLETED to skip vma_end_read(vma) for per-vma locks as= well > > > > > instead of introducing FAULT_FLAG_VMA_LOCK. I think that was your= idea > > > > > of reusing existing flags? > > > > Sorry, I meant VM_FAULT_VMA_UNLOCKED, not FAULT_FLAG_VMA_LOCK in th= e > > > > above reply. > > > > > > > > I took a closer look into using VM_FAULT_COMPLETED instead of > > > > VM_FAULT_VMA_UNLOCKED but when we fall back from per-vma lock to > > > > mmap_lock we need to retry with an indication that the per-vma lock > > > > was dropped. Returning (VM_FAULT_RETRY | VM_FAULT_COMPLETE) to > > > > indicate such state seems strange to me ("retry" and "complete" see= m > > > > > > Not relevant to this migration patch, but for the whole idea I was th= inking > > > whether it should just work if we simply: > > > > > > fault =3D handle_mm_fault(vma, address, flags | FAULT_FLAG_VM= A_LOCK, regs); > > > - vma_end_read(vma); > > > + if (!(fault & (VM_FAULT_RETRY | VM_FAULT_COMPLETED))) > > > + vma_end_read(vma); > > > > > > ? > > > > Today when we can't handle a page fault under per-vma locks we return > > VM_FAULT_RETRY, in which case per-vma lock is dropped and the fault is > > Oh I see what I missed. I think it may not be a good idea to reuse > VM_FAULT_RETRY just for that, because it makes VM_FAULT_RETRY even more > complicated - now it adds one more case where the lock is not released, > that's when PER_VMA even if !NOWAIT. > > > retried under mmap_lock. The condition you suggest above would not > > drop per-vma lock for VM_FAULT_RETRY case and would break the current > > fallback mechanism. > > However your suggestion gave me an idea. I could indicate that per-vma > > lock got dropped using vmf structure (like Matthew suggested before) > > and once handle_pte_fault(vmf) returns I could check if it returned > > VM_FAULT_RETRY but per-vma lock is still held. > > If that happens I can > > call vma_end_read() before returning from __handle_mm_fault(). That > > way any time handle_mm_fault() returns VM_FAULT_RETRY per-vma lock > > will be already released, so your condition in do_page_fault() will > > work correctly. That would eliminate the need for a new > > VM_FAULT_VMA_UNLOCKED flag. WDYT? > > Sounds good. > > So probably that's the major pain for now with the legacy fallback (I'll > have commented if I noticed it with the initial vma lock support..). I > assume that'll go away as long as we recover the VM_FAULT_RETRY semantics > to before. I think so. With that change getting VM_FAULT_RETRY in do_page_fault() will guarantee that per-vma lock was dropped. Is that what you mean? > > -- > Peter Xu > > -- > To unsubscribe from this group and stop receiving emails from it, send an= email to kernel-team+unsubscribe@android.com. >