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 5202DC77B7E for ; Tue, 2 May 2023 16:36:16 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E89A8900006; Tue, 2 May 2023 12:36:15 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id E39D3900002; Tue, 2 May 2023 12:36:15 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D51AE900006; Tue, 2 May 2023 12:36:15 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from mail-yw1-f179.google.com (mail-yw1-f179.google.com [209.85.128.179]) by kanga.kvack.org (Postfix) with ESMTP id B153F900002 for ; Tue, 2 May 2023 12:36:15 -0400 (EDT) Received: by mail-yw1-f179.google.com with SMTP id 00721157ae682-55a1462f9f6so28118397b3.3 for ; Tue, 02 May 2023 09:36:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1683045375; x=1685637375; 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=PtHOXsoZHVHTmGblvz+PcxSG2DNLj2mPGHa/Jp7hssk=; b=b++Vx6hWqbLjpG9pnp5Ijs00NxZ1mHvxjgv1ePpwdZx2ULh2lbwRETeHpoJq0keTl1 CDQHvn4OX37rIDwpXen1KFevJ0EcM7ietpn5afxAn27BQAPRoX4jmGjKGx+gZ91c4zph 7D8QwN0qu6fhoe5vtUQU7vYVq/dAcwBcdQ8V54c8LxkZ2b4Vo2+IcIc2qNsJdaBaYUFs NYiPXrJRcYiwXtB4gihMnOTQCmvyeYrYPVieyZLwhhb9KwC0t7s5IIo53/J+dNVMQrrk ALJpCPG/A+NV3VOe7EfJIKwejwuAf29e4p2u/Ss6sQYZHKyEP2h4QpAvgdplxvbVgLVu 5AWQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1683045375; x=1685637375; 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=PtHOXsoZHVHTmGblvz+PcxSG2DNLj2mPGHa/Jp7hssk=; b=dK1JtrH3ruXjddHZN3gBce9aayFumcM25LIEF+kDeclC/juN5RpS/ykUfxLDo0OgRR jhsquBYolHw5uA/dcauhEE6BIvJIy5JB3PXBy4x9J6KUureOJgP199B2Ol0Zb8vi4IYP 3H1ma3Op44siKv3DSLU8mHwSYqbpecyEON3JyfImU8RhZ//CEGVSiUBnyDAweqUnEnbD Gu3Hl1UuCspjwJmA1As+Izzf+n45sf/IPoo/VEbf3OSNtiOosgBmwBCCu6DgN/+BjSn7 0dyUFYeDxqYLYOWS2qwg5iufRV9pxhBw+tdkqGu7otNv1QcA6sqaDVsvxfuq4iirFdMp V0oQ== X-Gm-Message-State: AC+VfDwCzsUku7peQu+FQkPxTbD6ituxhl3E3haVip7aQHGoYlz/nUbE 36j4vqPAwDM0S3CYDICFL2VfH34XFPrvCHP8WXN/Mg== X-Google-Smtp-Source: ACHHUZ73XlgT3bgGX5m8siCZEgkQHLsvJiAYZQGGcfObRt8oITj9J8UmuFb+PbbPPfobFj5Vtk4gYJaOsQYYHPRraxQ= X-Received: by 2002:a05:6902:110e:b0:b99:36ff:d530 with SMTP id o14-20020a056902110e00b00b9936ffd530mr21584727ybu.30.1683045374954; Tue, 02 May 2023 09:36:14 -0700 (PDT) MIME-Version: 1.0 References: <20230501175025.36233-1-surenb@google.com> In-Reply-To: From: Suren Baghdasaryan Date: Tue, 2 May 2023 09:36:03 -0700 Message-ID: Subject: Re: [PATCH 1/3] mm: handle swap page faults under VMA lock if page is uncontended To: Matthew Wilcox Cc: akpm@linux-foundation.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, 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 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 Tue, May 2, 2023 at 8:03=E2=80=AFAM Matthew Wilcox = wrote: > > On Mon, May 01, 2023 at 10:04:56PM -0700, Suren Baghdasaryan wrote: > > On Mon, May 1, 2023 at 8:22=E2=80=AFPM Matthew Wilcox wrote: > > > > > > On Mon, May 01, 2023 at 07:30:13PM -0700, Suren Baghdasaryan wrote: > > > > On Mon, May 1, 2023 at 7:02=E2=80=AFPM Matthew Wilcox wrote: > > > > > > > > > > On Mon, May 01, 2023 at 10:50:23AM -0700, Suren Baghdasaryan wrot= e: > > > > > > +++ b/mm/memory.c > > > > > > @@ -3711,11 +3711,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) { > > > > > > - ret =3D VM_FAULT_RETRY; > > > > > > - goto out; > > > > > > - } > > > > > > - > > > > > > entry =3D pte_to_swp_entry(vmf->orig_pte); > > > > > > if (unlikely(non_swap_entry(entry))) { > > > > > > if (is_migration_entry(entry)) { > > > > > > > > > > You're missing the necessary fallback in the (!folio) case. > > > > > swap_readpage() is synchronous and will sleep. > > > > > > > > True, but is it unsafe to do that under VMA lock and has to be done > > > > under mmap_lock? > > > > > > ... you were the one arguing that we didn't want to wait for I/O with > > > the VMA lock held? > > > > Well, that discussion was about waiting in folio_lock_or_retry() with > > the lock being held. I argued against it because currently we drop > > mmap_lock lock before waiting, so if we don't drop VMA lock we would > > be changing the current behavior which might introduce new > > regressions. In the case of swap_readpage and swapin_readahead we > > already wait with mmap_lock held, so waiting with VMA lock held does > > not introduce new problems (unless there is a need to hold mmap_lock). > > > > That said, you are absolutely correct that this situation can be > > improved by dropping the lock in these cases too. I just didn't want > > to attack everything at once. I believe after we agree on the approach > > implemented in https://lore.kernel.org/all/20230501175025.36233-3-suren= b@google.com > > for dropping the VMA lock before waiting, these cases can be added > > easier. Does that make sense? > > OK, I looked at this path some more, and I think we're fine. This > patch is only called for SWP_SYNCHRONOUS_IO which is only set for > QUEUE_FLAG_SYNCHRONOUS devices, which are brd, zram and nvdimms > (both btt and pmem). So the answer is that we don't sleep in this > path, and there's no need to drop the lock. Yes but swapin_readahead does sleep, so I'll have to handle that case too after this.