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 C564DEB64D9 for ; Tue, 27 Jun 2023 15:41:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5C13B8D0002; Tue, 27 Jun 2023 11:41:47 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 571218D0001; Tue, 27 Jun 2023 11:41:47 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 412078D0002; Tue, 27 Jun 2023 11:41:47 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 2EBC58D0001 for ; Tue, 27 Jun 2023 11:41:47 -0400 (EDT) Received: from smtpin04.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 057BFC0194 for ; Tue, 27 Jun 2023 15:41:47 +0000 (UTC) X-FDA: 80948943054.04.31D511F Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf24.hostedemail.com (Postfix) with ESMTP id BB8FF180009 for ; Tue, 27 Jun 2023 15:41:44 +0000 (UTC) Authentication-Results: imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=XEDUG4LK; spf=pass (imf24.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687880504; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=uy66bIRInAw2b4KtURGbA1Gu6w2fF09+WZJT4/vEFBM=; b=CGqMXh93Y4r+ObR1KnDbooJIPgKPqTo0ek7BGfM0sqLArWW5aHXd+zRrJvW7eQOrY7qz1v vY8by5t+cCIrnFn0vcJ7FabTeVERBpS9/hRQs1qcwDvPMtWVc5CAZKqc40SmmoR/tWM+Sl /uoCZCMgH19KLiIbjNCPSFlN/e+dCr4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687880504; a=rsa-sha256; cv=none; b=aw0kBPK7x2FLe+3p7MwrcV3c1hdJZIloW+c81CkgPN92RlM7g3JE9+n52Q2Q+bw1C/PflT +ODhArgRdNbl0FUS5LRfRtjfLULxiWrsGx54JuTl5DIytX7yomJLXqKmc5Wal0X0rI/XGI j3KgpCaL4afeEgWeD3I1cWIYjLChZq8= ARC-Authentication-Results: i=1; imf24.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=XEDUG4LK; spf=pass (imf24.hostedemail.com: domain of peterx@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=peterx@redhat.com; dmarc=pass (policy=none) header.from=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1687880504; 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=uy66bIRInAw2b4KtURGbA1Gu6w2fF09+WZJT4/vEFBM=; b=XEDUG4LK1G1JrHxn9cr7Y5IjgRxQp30xsa/R21/ct7JhB/MRzHvNjFG0uQTyNOK3EWhD3H sYi3Df6ZA4/sz/Qr8i64zS7aETa/+xxylW9ntiYwTbgE+k/7nWcO060cDbAXCzuNF+z5pu 4EcCYAMvYL/142t2bgr0WQdCzg3jKzc= Received: from mail-qv1-f69.google.com (mail-qv1-f69.google.com [209.85.219.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-647-JBkSPQZYNY-22v3BT0YosA-1; Tue, 27 Jun 2023 11:41:40 -0400 X-MC-Unique: JBkSPQZYNY-22v3BT0YosA-1 Received: by mail-qv1-f69.google.com with SMTP id 6a1803df08f44-6340023ffbbso7690126d6.1 for ; Tue, 27 Jun 2023 08:41:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687880500; x=1690472500; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=uy66bIRInAw2b4KtURGbA1Gu6w2fF09+WZJT4/vEFBM=; b=WxbCA0CNx1zdAnO0kSo2mKiRdTY/C5kpmwlZDXP/NxfhAc/qHA5JiuJgJREAiYwJIU AzMuAitk69RY/iVefOQDdSzpOXA7FTrAeB5qodpnBNs26MP41Tni2s9OXsA5CmiuUjpJ g3mYtUoSeHLA5/gh0v+KINIQjBkIFnY3CyArsW/+e8xIJD4oc03trBIA5z7W81TxAj9i SfguON/81ExMvFQaKAHMbUn1MedUEpR8wEdDuPBfv7OWICWRjKgL+7b3lgS0A+NLzefV qrB3ki7OTo3sj/0A2rfB/QdXTB3gYTLbCsnU84DjaNS2RF8QyWopgWEgJRQbzT79VwCU Ikcw== X-Gm-Message-State: AC+VfDzKP7vSZltGgrJ1fU2gOb7TGWcDjSeDnLn/22PIIbCtHieEz8RF qN5OmEF58InjMaw624CfIGA6/eFwPIMCgIw4lj7ul1V/w2oiiWuVp7vA8ZsdGW5wh8tgnkyZJ4Z db/nW6izEAD4EUkjGFqI= X-Received: by 2002:ad4:5c65:0:b0:61b:2111:c2e2 with SMTP id i5-20020ad45c65000000b0061b2111c2e2mr2598585qvh.2.1687880500048; Tue, 27 Jun 2023 08:41:40 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ4ei3OS4UUP1x3N+KqQ03JAvCrKYZppqMcD8i/CjQVzJOEhilHL4mwwEgm1o4pNkAC5EYzv1A== X-Received: by 2002:ad4:5c65:0:b0:61b:2111:c2e2 with SMTP id i5-20020ad45c65000000b0061b2111c2e2mr2598554qvh.2.1687880499770; Tue, 27 Jun 2023 08:41:39 -0700 (PDT) Received: from x1n (cpe5c7695f3aee0-cm5c7695f3aede.cpe.net.cable.rogers.com. [99.254.144.39]) by smtp.gmail.com with ESMTPSA id a17-20020a0ccdd1000000b005dd8b9345b9sm4671640qvn.81.2023.06.27.08.41.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 27 Jun 2023 08:41:39 -0700 (PDT) Date: Tue, 27 Jun 2023 11:41:37 -0400 From: Peter Xu To: Suren Baghdasaryan 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 Subject: Re: [PATCH v3 6/8] mm: handle swap page faults under per-VMA lock Message-ID: References: <20230627042321.1763765-1-surenb@google.com> <20230627042321.1763765-7-surenb@google.com> MIME-Version: 1.0 In-Reply-To: <20230627042321.1763765-7-surenb@google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Rspamd-Queue-Id: BB8FF180009 X-Rspam-User: X-Rspamd-Server: rspam11 X-Stat-Signature: zeijdggnjc39et7enrsiwmq4eg9aw6sy X-HE-Tag: 1687880504-201935 X-HE-Meta: U2FsdGVkX1+Sk3CPiLS19zU/ww8o+y8CRw3wZWVuvPIMrgs30sy4Ow7utsq8puL+2apunMBUx7twV92Ls25UJ60NxRG1JHZcjEuIqq0L5EEjrGcr0X7uXkW/6Ynr9Yb8sKSwGJODOn9GX850gvGXKWfVo2My7BOIERJsxIL8imXKqVu1pGZsprqK16PAbRK8BnOy4VfDz41URKTZHQOkDOy4qdQxX4h0cg+CkeYq3NMtt8jNXPuFfNk8S2FY7wfLLn8/zP+PowAhdWkbVOvVml8RQWf/CkMmcAq0kxhNTL9tJrK3hd/I3f9K4upXDIgmDlg7l4igrPs4ZFEQfxegpzkXiJxRith7W2c6MCT07M0nJ2paPbegzs/2CeILiHtmjqQvxX4il+lvr8NZBansNSky9hvDpwY0FznejX2ojkK239JGI/QEc3bEqXHhhl9u4fMFKiRp9uKRAX/9PpP6ktklr/ndR/A+Yv6pxijWTjQ2lSzoJvNm5VlxAg32x6h6MiDl6zZINnwVU5IHQCwCuQFG1stpeSFZmuTPPIbcHFT12HK+8QUhEec0ErbQvJuLhBiuYOxkUj2l/6SeXSE8Txvk34zU411oaHLP4PnUzkwMiOhrOPyyq94u90QiGDvZomwy4TBYuohrk7dkfnU53Kj3wKvKyqPvycLtimBBCX5MIHxBrF7t6Vc607K3YQgIPqrR/Nex3yfJJLuSxV6xIsUWuGoHFtc7pAz8EtcbaZxsAVv4N1VouF/wqwPHhWz56y84SX6vMslALqZfFuZ5Jo0eR/rRUuZy75RF2klfwGpdm4fVwNe/N207Y7q8Me0+YMPCIMeURHUnyVHQ/SFkT0AfWZH3hnyispKp7oG91ApQFtM4L+wNGCgpopfeF9RqaPCW68sQgjye/BPyMqqorXPJ/gmgMoGluemuKGi9lVf3Oo7BaPJmFxqIqtkXBsA8iueH60t6J/icszWIEmS jQwoMG27 T1r3kFxWZZjCKbpxwO1OzggZEgvYVqdfos6YuRHlKswvtFNyk+y/k8kTE70p3QONELj/99eij4LUxN0y8WPA4A2do2lb9d016RSp/mnRqI/2HUE9DO9IJjA1PRd4Ikkyo9pW/TSeO9KLGmgZ+x5oiZBtGjlxyzW5nqwDlQ4AZpABlRSfRM0P3GeXX4YqZ4zdt9IiGNHx3D08pZo1yYNDlnBLaawc2pX/mjGOOCdFHo3musXLiZGwZBhJW09rhe75nWnoDDaIjBljaF++zfXaB6qJ1Ee+vX67LwN7Gwn/owIDYhQMYkvxnOV0jTvgthoxKMkgpEKMQO1m5JxQ+LghLHYpG1NUyKm633zAFZ1uV7jFgX4wmQp86biovn8GefNjoZY9T+k1uEl0Xqz4= 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 Mon, Jun 26, 2023 at 09:23:19PM -0700, Suren Baghdasaryan wrote: > When page fault is handled under per-VMA lock protection, all swap page > faults are retried with mmap_lock because folio_lock_fault (formerly > known as folio_lock_or_retry) had to drop and reacquire mmap_lock > if folio could not be immediately locked. > Follow the same pattern as mmap_lock to drop per-VMA lock when waiting > for folio in folio_lock_fault and retrying once folio is available. > With this obstacle removed, enable do_swap_page to operate under > per-VMA lock protection. Drivers implementing ops->migrate_to_ram might > still rely on mmap_lock, therefore we have to fall back to mmap_lock in > that particular case. > Note that the only time do_swap_page calls synchronous swap_readpage > is when SWP_SYNCHRONOUS_IO is set, which is only set for > QUEUE_FLAG_SYNCHRONOUS devices: brd, zram and nvdimms (both btt and > pmem). Therefore we don't sleep in this path, and there's no need to > drop the mmap or per-VMA lock. > > Signed-off-by: Suren Baghdasaryan > --- > mm/filemap.c | 24 ++++++++++++++++-------- > mm/memory.c | 21 ++++++++++++++------- > 2 files changed, 30 insertions(+), 15 deletions(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 8ad06d69895b..683f11f244cd 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -1703,12 +1703,14 @@ static int __folio_lock_async(struct folio *folio, struct wait_page_queue *wait) > * Return values: > * 0 - folio is locked. > * VM_FAULT_RETRY - folio is not locked. > - * mmap_lock has been released (mmap_read_unlock(), unless flags had both > - * FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_RETRY_NOWAIT set, in > - * which case mmap_lock is still held. > + * FAULT_FLAG_LOCK_DROPPED bit in vmf flags will be set if mmap_lock or This "FAULT_FLAG_LOCK_DROPPED" should belong to that patch when introduced. But again I still think this flag as a whole with that patch is not needed and should be dropped, unless I miss something important.. > + * per-VMA lock got dropped. mmap_lock/per-VMA lock is dropped when > + * function fails to lock the folio, unless flags had both > + * FAULT_FLAG_ALLOW_RETRY and FAULT_FLAG_RETRY_NOWAIT set, in which case > + * the lock is still held. > * > * If neither ALLOW_RETRY nor KILLABLE are set, will always return 0 > - * with the folio locked and the mmap_lock unperturbed. > + * with the folio locked and the mmap_lock/per-VMA lock unperturbed. > */ > vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > { > @@ -1716,13 +1718,16 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > if (fault_flag_allow_retry_first(vmf->flags)) { > /* > - * CAUTION! In this case, mmap_lock is not released > - * even though return VM_FAULT_RETRY. > + * CAUTION! In this case, mmap_lock/per-VMA lock is not > + * released even though returning VM_FAULT_RETRY. > */ > if (vmf->flags & FAULT_FLAG_RETRY_NOWAIT) > return VM_FAULT_RETRY; > > - mmap_read_unlock(mm); > + if (vmf->flags & FAULT_FLAG_VMA_LOCK) > + vma_end_read(vmf->vma); > + else > + mmap_read_unlock(mm); > vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > if (vmf->flags & FAULT_FLAG_KILLABLE) > folio_wait_locked_killable(folio); > @@ -1735,7 +1740,10 @@ vm_fault_t __folio_lock_fault(struct folio *folio, struct vm_fault *vmf) > > ret = __folio_lock_killable(folio); > if (ret) { > - mmap_read_unlock(mm); > + if (vmf->flags & FAULT_FLAG_VMA_LOCK) > + vma_end_read(vmf->vma); > + else > + mmap_read_unlock(mm); > vmf->flags |= FAULT_FLAG_LOCK_DROPPED; > return VM_FAULT_RETRY; > } > diff --git a/mm/memory.c b/mm/memory.c > index 3c2acafcd7b6..5caaa4c66ea2 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -3712,11 +3712,6 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > if (!pte_unmap_same(vmf)) > goto out; > > - if (vmf->flags & FAULT_FLAG_VMA_LOCK) { So if with my imagination, here we'll already have the vma_read_end() and this patch will remove it, which makes sense. Then... > - ret = VM_FAULT_RETRY; > - goto out; > - } > - > entry = pte_to_swp_entry(vmf->orig_pte); > if (unlikely(non_swap_entry(entry))) { > if (is_migration_entry(entry)) { > @@ -3726,6 +3721,15 @@ vm_fault_t do_swap_page(struct vm_fault *vmf) > vmf->page = pfn_swap_entry_to_page(entry); > ret = remove_device_exclusive_entry(vmf); > } else if (is_device_private_entry(entry)) { > + if (vmf->flags & FAULT_FLAG_VMA_LOCK) { > + /* > + * migrate_to_ram is not yet ready to operate > + * under VMA lock. > + */ ... here we probably just do vma_read_end(), then... > + ret |= VM_FAULT_RETRY; > + goto out; > + } > + > vmf->page = pfn_swap_entry_to_page(entry); > vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, > vmf->address, &vmf->ptl); > @@ -5089,9 +5093,12 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, > /* > * In case of VM_FAULT_RETRY or VM_FAULT_COMPLETED we might > * be still holding per-VMA lock to keep the vma stable as long > - * as possible. Drop it before returning. > + * as possible. In this situation vmf.flags has > + * FAULT_FLAG_VMA_LOCK set and FAULT_FLAG_LOCK_DROPPED unset. > + * Drop the lock before returning when this happens. > */ > - if (vmf.flags & FAULT_FLAG_VMA_LOCK) > + if ((vmf.flags & (FAULT_FLAG_VMA_LOCK | FAULT_FLAG_LOCK_DROPPED)) == > + FAULT_FLAG_VMA_LOCK) > vma_end_read(vma); This whole chunk should have been dropped altogether with my comment in previous patch, iiuc, and it should be no-op anyway for swap case. For the real "waiting for page lock during swapin" phase we should always 100% release the vma lock in folio_lock_or_retry() - just like mmap lock. Thanks, > } > return ret; > -- > 2.41.0.178.g377b9f9a00-goog > -- Peter Xu