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 X-Spam-Level: X-Spam-Status: No, score=-13.3 required=3.0 tests=BAYES_00,DKIMWL_WL_MED, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9AD17C4743D for ; Sat, 5 Jun 2021 00:42:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 760CE610C9 for ; Sat, 5 Jun 2021 00:42:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231225AbhFEAoR (ORCPT ); Fri, 4 Jun 2021 20:44:17 -0400 Received: from mail-lj1-f181.google.com ([209.85.208.181]:33777 "EHLO mail-lj1-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229853AbhFEAoQ (ORCPT ); Fri, 4 Jun 2021 20:44:16 -0400 Received: by mail-lj1-f181.google.com with SMTP id o8so13826700ljp.0 for ; Fri, 04 Jun 2021 17:42:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=ojqlPcYN//lzjPCK23tdLtRIIYrgW9m/5kJ/xOjRInCF4R7iFC1/I0ZkVLmxGV8DXR MQWb07lHHZMwwI3mGCRqMAesfKm1mWvsT1fKDr2EyaZCEGoXBk5MRscqdZbVrfjd6byB UC2OAmXNTjZ1XXqU2QIkZUr+320CRdtEbj7InexW97stnDZxR/A2mRYURCjA4zPGoT8h zZKD7TJ6uy/9jzID2bu4rxwrKqn/bYNh5LMyz+lefblMFpIWrDYNy/UqqVnv9iY+GCYm zCIocFSTC5JEnil58sWiJDJ+0EtmDGYceal8ew26Jn/vZJQwmrzF2MxxEU2hsojtFjNK mK1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=f4767xQtN0Z1qY+IJ40DiwY3nZMrylHHcIrynuS15mJKJAkQMW6J6Jum04KW99+n1U TPmrQ16Qb47Qk69M7T0tvw0BpdVVehGy/HBWm9AW6tvUIlWrJsPU3WT/a9M7WJ+jvkw8 dM66gCf4F6+u7Fv25aju6T/smlpgEV51PSo9KPxd4/uAP7ODeHsx7SX14b7u1obAQjYg Tz2r6LNvS3VQTxgQbQHT66uTMhee9YFltP6OJhVzhdAseiweVcz8qQpEWP6jbf8dJaqG yrg+4NoA247ODIi/xeISC3X0eMfFFEezYCbn4AkWYgjKejUgMfLAe7J6j/xp1hnMaUaV ZzvA== X-Gm-Message-State: AOAM530wHJj5JM7hkQgVGe2Hc8IVMgNuQMahsruGGDVSPt7PIWa2yKio n0SE2UhZ8p744In0xT0WEhcppStTttra03tKzgnc2w== X-Google-Smtp-Source: ABdhPJz1RZbFciQV1FijFF60AoPQ2WRU7bGHglkLlwJI2jhHqLCKOZ4khT7GPCOGC1aZRVomGnuidnciWbK1dWihUSA= X-Received: by 2002:a05:651c:292:: with SMTP id b18mr5197181ljo.456.1622853674402; Fri, 04 Jun 2021 17:41:14 -0700 (PDT) MIME-Version: 1.0 References: <20210524132725.12697-1-apopple@nvidia.com> <20210524132725.12697-4-apopple@nvidia.com> <20210525183710.fa2m2sbfixnhz7g5@revolver> <20210604204934.sbspsmwdqdtmz73d@revolver> In-Reply-To: <20210604204934.sbspsmwdqdtmz73d@revolver> From: Shakeel Butt Date: Fri, 4 Jun 2021 17:41:03 -0700 Message-ID: Subject: Re: [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap To: Liam Howlett Cc: Alistair Popple , "linux-mm@kvack.org" , "akpm@linux-foundation.org" , "nouveau@lists.freedesktop.org" , "bskeggs@redhat.com" , "rcampbell@nvidia.com" , "linux-doc@vger.kernel.org" , "jhubbard@nvidia.com" , "bsingharora@gmail.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "hch@infradead.org" , "jglisse@redhat.com" , "willy@infradead.org" , "jgg@nvidia.com" , "peterx@redhat.com" , "hughd@google.com" , Christoph Hellwig Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Jun 4, 2021 at 1:49 PM Liam Howlett wrote: > > * Shakeel Butt [210525 19:45]: > > On Tue, May 25, 2021 at 11:40 AM Liam Howlett wrote: > > > > > [...] > > > > > > > > +/* > > > > + * Walks the vma's mapping a page and mlocks the page if any locked vma's are > > > > + * found. Once one is found the page is locked and the scan can be terminated. > > > > + */ > > > > > > Can you please add that this requires the mmap_sem() lock to the > > > comments? > > > > > > > Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct? > > > Doesn't the mlock_vma_page() require the mmap_sem() for reading? The > mm_struct in vma->vm_mm; > We are traversing all the vmas where this page is mapped of possibly different mm_structs. I don't think we want to take mmap_sem() of all those mm_structs. The commit b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked") removed exactly that. > > From what I can see, at least the following paths have mmap_lock held > for writing: > > munlock_vma_pages_range() from __do_munmap() > munlokc_vma_pages_range() from remap_file_pages() > The following path does not hold mmap_sem: exit_mmap() -> munlock_vma_pages_all() -> munlock_vma_pages_range(). I would really suggest all to carefully read the commit message of b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked"). Particularly the following paragraph: ... Vlastimil Babka points out another race which this patch protects against. try_to_unmap_one() might reach its mlock_vma_page() TestSetPageMlocked a moment after munlock_vma_pages_all() did its Phase 1 TestClearPageMlocked: leaving PageMlocked and unevictable when it should be evictable. mmap_sem is ineffective because exit_mmap() does not hold it; page lock ineffective because __munlock_pagevec() only takes it afterwards, in Phase 2; pte lock is effective because __munlock_pagevec_fill() takes it to get the page, after VM_LOCKED was cleared from vm_flags, so visible to try_to_unmap_one. ... Alistair, please bring back the VM_LOCKED check with pte lock held and the comment "Holding pte lock, we do *not* need mmap_lock here". One positive outcome of this cleanup patch is the removal of unnecessary invalidation (unmapping for kvm case) of secondary mmus. 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 X-Spam-Level: X-Spam-Status: No, score=-11.7 required=3.0 tests=BAYES_00,DATE_IN_PAST_03_06, DKIMWL_WL_MED,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_IN_DEF_DKIM_WL autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6959AC47082 for ; Sat, 5 Jun 2021 06:18:25 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 0063E61244 for ; Sat, 5 Jun 2021 06:18:24 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 0063E61244 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9284E6B006C; Sat, 5 Jun 2021 02:18:24 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8FF376B006E; Sat, 5 Jun 2021 02:18:24 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7A0646B0070; Sat, 5 Jun 2021 02:18:24 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0093.hostedemail.com [216.40.44.93]) by kanga.kvack.org (Postfix) with ESMTP id 4A6836B006C for ; Sat, 5 Jun 2021 02:18:24 -0400 (EDT) Received: from smtpin28.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id DCD206135 for ; Sat, 5 Jun 2021 06:18:23 +0000 (UTC) X-FDA: 78218665686.28.649B927 Received: from mail-ej1-f54.google.com (mail-ej1-f54.google.com [209.85.218.54]) by imf15.hostedemail.com (Postfix) with ESMTP id 759E6A00025C for ; Sat, 5 Jun 2021 06:17:44 +0000 (UTC) Received: by mail-ej1-f54.google.com with SMTP id b9so17733026ejc.13 for ; Fri, 04 Jun 2021 23:17:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=ojqlPcYN//lzjPCK23tdLtRIIYrgW9m/5kJ/xOjRInCF4R7iFC1/I0ZkVLmxGV8DXR MQWb07lHHZMwwI3mGCRqMAesfKm1mWvsT1fKDr2EyaZCEGoXBk5MRscqdZbVrfjd6byB UC2OAmXNTjZ1XXqU2QIkZUr+320CRdtEbj7InexW97stnDZxR/A2mRYURCjA4zPGoT8h zZKD7TJ6uy/9jzID2bu4rxwrKqn/bYNh5LMyz+lefblMFpIWrDYNy/UqqVnv9iY+GCYm zCIocFSTC5JEnil58sWiJDJ+0EtmDGYceal8ew26Jn/vZJQwmrzF2MxxEU2hsojtFjNK mK1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=AZm8Ijdv3AoYieY84dCbQvIv1TpO+yMuis0G3LWVrtWKXKVNgTAkKB2T/wLcmavQvA dEAU3Hhr8hJhWtdELgeMPF6jsuW2XdDa82cR3tKxZ92fzkX2ht/P3JIAuZj+9MhRzP8W qlNDfD4FimkejsfTl4BypxfvfZyiV1aSCNiFsjYHuMizvGepYG6PVxaqL6yW1Jbg5mdI Yrw7Rk5uBHcSFXskgRbXNEjmcplSeDwMTPUvK1YcjbXTRZNXZDg9l0DySIJBaE1frqbW FxCuuZSESkHUqrq6yBwkS9g9Q+nD2zzehn8ZQNPE1S3K6Yrn0nLmeGAgFky2RDogoITB jNhQ== X-Gm-Message-State: AOAM531zXDLAdEIpWIlbR2pEME7970+R0E6uBR1rdYMNVUpKREPU9eQL frOciYX8G63nNZri6RldaEh2/UHHugjJe76JaoIflyBBCq069A== X-Google-Smtp-Source: ABdhPJz1RZbFciQV1FijFF60AoPQ2WRU7bGHglkLlwJI2jhHqLCKOZ4khT7GPCOGC1aZRVomGnuidnciWbK1dWihUSA= X-Received: by 2002:a05:651c:292:: with SMTP id b18mr5197181ljo.456.1622853674402; Fri, 04 Jun 2021 17:41:14 -0700 (PDT) MIME-Version: 1.0 References: <20210524132725.12697-1-apopple@nvidia.com> <20210524132725.12697-4-apopple@nvidia.com> <20210525183710.fa2m2sbfixnhz7g5@revolver> <20210604204934.sbspsmwdqdtmz73d@revolver> In-Reply-To: <20210604204934.sbspsmwdqdtmz73d@revolver> From: Shakeel Butt Date: Fri, 4 Jun 2021 17:41:03 -0700 Message-ID: Subject: Re: [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap To: Liam Howlett Cc: Alistair Popple , "linux-mm@kvack.org" , "akpm@linux-foundation.org" , "nouveau@lists.freedesktop.org" , "bskeggs@redhat.com" , "rcampbell@nvidia.com" , "linux-doc@vger.kernel.org" , "jhubbard@nvidia.com" , "bsingharora@gmail.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "hch@infradead.org" , "jglisse@redhat.com" , "willy@infradead.org" , "jgg@nvidia.com" , "peterx@redhat.com" , "hughd@google.com" , Christoph Hellwig Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf15.hostedemail.com; dkim=pass header.d=google.com header.s=20161025 header.b=ojqlPcYN; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf15.hostedemail.com: domain of shakeelb@google.com designates 209.85.218.54 as permitted sender) smtp.mailfrom=shakeelb@google.com X-Stat-Signature: myjumpr4hq44kdyntwgx7ju8ybd63uu8 X-Rspamd-Queue-Id: 759E6A00025C X-Rspamd-Server: rspam02 X-HE-Tag: 1622873864-236486 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 Fri, Jun 4, 2021 at 1:49 PM Liam Howlett wrote: > > * Shakeel Butt [210525 19:45]: > > On Tue, May 25, 2021 at 11:40 AM Liam Howlett wrote: > > > > > [...] > > > > > > > > +/* > > > > + * Walks the vma's mapping a page and mlocks the page if any locked vma's are > > > > + * found. Once one is found the page is locked and the scan can be terminated. > > > > + */ > > > > > > Can you please add that this requires the mmap_sem() lock to the > > > comments? > > > > > > > Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct? > > > Doesn't the mlock_vma_page() require the mmap_sem() for reading? The > mm_struct in vma->vm_mm; > We are traversing all the vmas where this page is mapped of possibly different mm_structs. I don't think we want to take mmap_sem() of all those mm_structs. The commit b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked") removed exactly that. > > From what I can see, at least the following paths have mmap_lock held > for writing: > > munlock_vma_pages_range() from __do_munmap() > munlokc_vma_pages_range() from remap_file_pages() > The following path does not hold mmap_sem: exit_mmap() -> munlock_vma_pages_all() -> munlock_vma_pages_range(). I would really suggest all to carefully read the commit message of b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked"). Particularly the following paragraph: ... Vlastimil Babka points out another race which this patch protects against. try_to_unmap_one() might reach its mlock_vma_page() TestSetPageMlocked a moment after munlock_vma_pages_all() did its Phase 1 TestClearPageMlocked: leaving PageMlocked and unevictable when it should be evictable. mmap_sem is ineffective because exit_mmap() does not hold it; page lock ineffective because __munlock_pagevec() only takes it afterwards, in Phase 2; pte lock is effective because __munlock_pagevec_fill() takes it to get the page, after VM_LOCKED was cleared from vm_flags, so visible to try_to_unmap_one. ... Alistair, please bring back the VM_LOCKED check with pte lock held and the comment "Holding pte lock, we do *not* need mmap_lock here". One positive outcome of this cleanup patch is the removal of unnecessary invalidation (unmapping for kvm case) of secondary mmus. 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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id F06A2C47096 for ; Sun, 6 Jun 2021 17:34:26 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B74DD613D4 for ; Sun, 6 Jun 2021 17:34:26 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B74DD613D4 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=nouveau-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D46306E079; Sun, 6 Jun 2021 17:34:23 +0000 (UTC) Received: from mail-lj1-x229.google.com (mail-lj1-x229.google.com [IPv6:2a00:1450:4864:20::229]) by gabe.freedesktop.org (Postfix) with ESMTPS id 3EFB86E43B for ; Sat, 5 Jun 2021 00:41:16 +0000 (UTC) Received: by mail-lj1-x229.google.com with SMTP id w15so13734295ljo.10 for ; Fri, 04 Jun 2021 17:41:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=ojqlPcYN//lzjPCK23tdLtRIIYrgW9m/5kJ/xOjRInCF4R7iFC1/I0ZkVLmxGV8DXR MQWb07lHHZMwwI3mGCRqMAesfKm1mWvsT1fKDr2EyaZCEGoXBk5MRscqdZbVrfjd6byB UC2OAmXNTjZ1XXqU2QIkZUr+320CRdtEbj7InexW97stnDZxR/A2mRYURCjA4zPGoT8h zZKD7TJ6uy/9jzID2bu4rxwrKqn/bYNh5LMyz+lefblMFpIWrDYNy/UqqVnv9iY+GCYm zCIocFSTC5JEnil58sWiJDJ+0EtmDGYceal8ew26Jn/vZJQwmrzF2MxxEU2hsojtFjNK mK1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=B7qmQQoVR9jmIwn+2jzyZ4s5BuMRgy70h0SWa7fM8dPl4Igf5k4LfjBnDbnzU15KfM WilK05OS21xXRQKktn1s4kYiFWcMw611aIVSXBmqoonhI29aEJG7A5CFxGVf8pDL6MeT 9S2i2R2CLiiLBkrFTB+jb9hmymNzq/KKFX+Gk2zUSaU7ELEDOay5G0y+yvzwcXsrQado l/TYkY2d2OsnHFKOkxjtwkr1bZdRrnX4zHljCWzPbFhCdbNaEFE4iBXCcDO1EZ5QHdB7 R5KcYEH5lStzssmLY/naMVSEyk6k8tsBxGsoOYbBp65lJrrCGNe5rQpwYzLafDylX7CW m7bw== X-Gm-Message-State: AOAM5310VJNIQhd8EWS/eh/LBWjV4Uw3bWYgsx4pUIhr7qH7p+nRZy9n 7F7g4IAbtFmBKERGay8Wg5EGLm0rmHK4UU5uPSJLqA== X-Google-Smtp-Source: ABdhPJz1RZbFciQV1FijFF60AoPQ2WRU7bGHglkLlwJI2jhHqLCKOZ4khT7GPCOGC1aZRVomGnuidnciWbK1dWihUSA= X-Received: by 2002:a05:651c:292:: with SMTP id b18mr5197181ljo.456.1622853674402; Fri, 04 Jun 2021 17:41:14 -0700 (PDT) MIME-Version: 1.0 References: <20210524132725.12697-1-apopple@nvidia.com> <20210524132725.12697-4-apopple@nvidia.com> <20210525183710.fa2m2sbfixnhz7g5@revolver> <20210604204934.sbspsmwdqdtmz73d@revolver> In-Reply-To: <20210604204934.sbspsmwdqdtmz73d@revolver> From: Shakeel Butt Date: Fri, 4 Jun 2021 17:41:03 -0700 Message-ID: To: Liam Howlett X-Mailman-Approved-At: Sun, 06 Jun 2021 17:34:23 +0000 Subject: Re: [Nouveau] [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap X-BeenThere: nouveau@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Nouveau development list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "rcampbell@nvidia.com" , "willy@infradead.org" , "linux-doc@vger.kernel.org" , "nouveau@lists.freedesktop.org" , "bsingharora@gmail.com" , Alistair Popple , "hughd@google.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "peterx@redhat.com" , "hch@infradead.org" , "linux-mm@kvack.org" , "bskeggs@redhat.com" , "jgg@nvidia.com" , "akpm@linux-foundation.org" , Christoph Hellwig Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: nouveau-bounces@lists.freedesktop.org Sender: "Nouveau" On Fri, Jun 4, 2021 at 1:49 PM Liam Howlett wrote: > > * Shakeel Butt [210525 19:45]: > > On Tue, May 25, 2021 at 11:40 AM Liam Howlett wrote: > > > > > [...] > > > > > > > > +/* > > > > + * Walks the vma's mapping a page and mlocks the page if any locked vma's are > > > > + * found. Once one is found the page is locked and the scan can be terminated. > > > > + */ > > > > > > Can you please add that this requires the mmap_sem() lock to the > > > comments? > > > > > > > Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct? > > > Doesn't the mlock_vma_page() require the mmap_sem() for reading? The > mm_struct in vma->vm_mm; > We are traversing all the vmas where this page is mapped of possibly different mm_structs. I don't think we want to take mmap_sem() of all those mm_structs. The commit b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked") removed exactly that. > > From what I can see, at least the following paths have mmap_lock held > for writing: > > munlock_vma_pages_range() from __do_munmap() > munlokc_vma_pages_range() from remap_file_pages() > The following path does not hold mmap_sem: exit_mmap() -> munlock_vma_pages_all() -> munlock_vma_pages_range(). I would really suggest all to carefully read the commit message of b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked"). Particularly the following paragraph: ... Vlastimil Babka points out another race which this patch protects against. try_to_unmap_one() might reach its mlock_vma_page() TestSetPageMlocked a moment after munlock_vma_pages_all() did its Phase 1 TestClearPageMlocked: leaving PageMlocked and unevictable when it should be evictable. mmap_sem is ineffective because exit_mmap() does not hold it; page lock ineffective because __munlock_pagevec() only takes it afterwards, in Phase 2; pte lock is effective because __munlock_pagevec_fill() takes it to get the page, after VM_LOCKED was cleared from vm_flags, so visible to try_to_unmap_one. ... Alistair, please bring back the VM_LOCKED check with pte lock held and the comment "Holding pte lock, we do *not* need mmap_lock here". One positive outcome of this cleanup patch is the removal of unnecessary invalidation (unmapping for kvm case) of secondary mmus. _______________________________________________ Nouveau mailing list Nouveau@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/nouveau 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 X-Spam-Level: X-Spam-Status: No, score=-3.5 required=3.0 tests=BAYES_00,DKIM_ADSP_CUSTOM_MED, DKIM_INVALID,DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 36356C4743C for ; Sat, 5 Jun 2021 00:41:18 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id CFD2F61359 for ; Sat, 5 Jun 2021 00:41:17 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org CFD2F61359 Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=dri-devel-bounces@lists.freedesktop.org Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id D38426E43A; Sat, 5 Jun 2021 00:41:16 +0000 (UTC) Received: from mail-lj1-x236.google.com (mail-lj1-x236.google.com [IPv6:2a00:1450:4864:20::236]) by gabe.freedesktop.org (Postfix) with ESMTPS id 2FA366E43A for ; Sat, 5 Jun 2021 00:41:16 +0000 (UTC) Received: by mail-lj1-x236.google.com with SMTP id p20so13746623ljj.8 for ; Fri, 04 Jun 2021 17:41:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=ojqlPcYN//lzjPCK23tdLtRIIYrgW9m/5kJ/xOjRInCF4R7iFC1/I0ZkVLmxGV8DXR MQWb07lHHZMwwI3mGCRqMAesfKm1mWvsT1fKDr2EyaZCEGoXBk5MRscqdZbVrfjd6byB UC2OAmXNTjZ1XXqU2QIkZUr+320CRdtEbj7InexW97stnDZxR/A2mRYURCjA4zPGoT8h zZKD7TJ6uy/9jzID2bu4rxwrKqn/bYNh5LMyz+lefblMFpIWrDYNy/UqqVnv9iY+GCYm zCIocFSTC5JEnil58sWiJDJ+0EtmDGYceal8ew26Jn/vZJQwmrzF2MxxEU2hsojtFjNK mK1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=S0RNl1ValKKRLQbUx9zapSSxQl8HE3O+Grx55QC8BB8=; b=Iiu86uRF++AFsfgaRxklMZsPv5tjUAMZ0t/5pbF9eW6lkGKl0l4miuqb2V/5VVBifK yrQ+GB3XbOQ9vNO6bVsrXriLm+haLV1HBr6acKNDs88Sn8afvUcQi3NVPYO0+PpAzN6u UlBQ0ybNFOK2ga7Nf+kekiwbjXEE4aLQzaESKOT1egQcZt7cOkJ0W+oj4BtMnZT4jwGr xLlF3Cu2jPovBPoE5NiagM+GDUgnRkhFH3cvjdTPd2jS6mbGTF3Ht7FGADEkgBV+UPGK E2gTqqpSn7WPv9c49IgrqLipCcNxTGGhygt3xagZOO2bZ0SJWJLqjpBQVteNRaK5mSU1 gvzg== X-Gm-Message-State: AOAM530UkdYa0P63pRHwu7hTEp6f0i2LQKCQ+hv091h/Ti+Aa8GZeJpO fDILrdHvui/tm4NzVQw+cjzfEVnx0x/nrxpoASzaKQ== X-Google-Smtp-Source: ABdhPJz1RZbFciQV1FijFF60AoPQ2WRU7bGHglkLlwJI2jhHqLCKOZ4khT7GPCOGC1aZRVomGnuidnciWbK1dWihUSA= X-Received: by 2002:a05:651c:292:: with SMTP id b18mr5197181ljo.456.1622853674402; Fri, 04 Jun 2021 17:41:14 -0700 (PDT) MIME-Version: 1.0 References: <20210524132725.12697-1-apopple@nvidia.com> <20210524132725.12697-4-apopple@nvidia.com> <20210525183710.fa2m2sbfixnhz7g5@revolver> <20210604204934.sbspsmwdqdtmz73d@revolver> In-Reply-To: <20210604204934.sbspsmwdqdtmz73d@revolver> From: Shakeel Butt Date: Fri, 4 Jun 2021 17:41:03 -0700 Message-ID: Subject: Re: [PATCH v9 03/10] mm/rmap: Split try_to_munlock from try_to_unmap To: Liam Howlett Content-Type: text/plain; charset="UTF-8" X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "rcampbell@nvidia.com" , "willy@infradead.org" , "linux-doc@vger.kernel.org" , "nouveau@lists.freedesktop.org" , "bsingharora@gmail.com" , Alistair Popple , "hughd@google.com" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , "peterx@redhat.com" , "hch@infradead.org" , "linux-mm@kvack.org" , "jglisse@redhat.com" , "bskeggs@redhat.com" , "jgg@nvidia.com" , "jhubbard@nvidia.com" , "akpm@linux-foundation.org" , Christoph Hellwig Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" On Fri, Jun 4, 2021 at 1:49 PM Liam Howlett wrote: > > * Shakeel Butt [210525 19:45]: > > On Tue, May 25, 2021 at 11:40 AM Liam Howlett wrote: > > > > > [...] > > > > > > > > +/* > > > > + * Walks the vma's mapping a page and mlocks the page if any locked vma's are > > > > + * found. Once one is found the page is locked and the scan can be terminated. > > > > + */ > > > > > > Can you please add that this requires the mmap_sem() lock to the > > > comments? > > > > > > > Why does this require mmap_sem() lock? Also mmap_sem() lock of which mm_struct? > > > Doesn't the mlock_vma_page() require the mmap_sem() for reading? The > mm_struct in vma->vm_mm; > We are traversing all the vmas where this page is mapped of possibly different mm_structs. I don't think we want to take mmap_sem() of all those mm_structs. The commit b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked") removed exactly that. > > From what I can see, at least the following paths have mmap_lock held > for writing: > > munlock_vma_pages_range() from __do_munmap() > munlokc_vma_pages_range() from remap_file_pages() > The following path does not hold mmap_sem: exit_mmap() -> munlock_vma_pages_all() -> munlock_vma_pages_range(). I would really suggest all to carefully read the commit message of b87537d9e2fe ("mm: rmap use pte lock not mmap_sem to set PageMlocked"). Particularly the following paragraph: ... Vlastimil Babka points out another race which this patch protects against. try_to_unmap_one() might reach its mlock_vma_page() TestSetPageMlocked a moment after munlock_vma_pages_all() did its Phase 1 TestClearPageMlocked: leaving PageMlocked and unevictable when it should be evictable. mmap_sem is ineffective because exit_mmap() does not hold it; page lock ineffective because __munlock_pagevec() only takes it afterwards, in Phase 2; pte lock is effective because __munlock_pagevec_fill() takes it to get the page, after VM_LOCKED was cleared from vm_flags, so visible to try_to_unmap_one. ... Alistair, please bring back the VM_LOCKED check with pte lock held and the comment "Holding pte lock, we do *not* need mmap_lock here". One positive outcome of this cleanup patch is the removal of unnecessary invalidation (unmapping for kvm case) of secondary mmus.