linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Will Deacon <will@kernel.org>
Cc: Jan Kara <jack@suse.cz>, Minchan Kim <minchan@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Linux-MM <linux-mm@kvack.org>,
	Vinayak Menon <vinmenon@codeaurora.org>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Android Kernel Team <kernel-team@android.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting
Date: Wed, 9 Dec 2020 09:58:12 -0800	[thread overview]
Message-ID: <CAHk-=wgos=vgteG52=J=rVSeq6-Y2g2+Kn1=xV=wYjVzM6O9UQ@mail.gmail.com> (raw)
In-Reply-To: <20201209163950.8494-2-will@kernel.org>

On Wed, Dec 9, 2020 at 8:40 AM Will Deacon <will@kernel.org> wrote:
>
> @@ -3978,8 +3994,17 @@ static vm_fault_t do_fault_around(struct vm_fault *vmf)
>
>         /* check if the page fault is solved */
>         vmf->pte -= (vmf->address >> PAGE_SHIFT) - (address >> PAGE_SHIFT);
> -       if (!pte_none(*vmf->pte))
> -               ret = VM_FAULT_NOPAGE;
> +       if (pte_none(*vmf->pte))
> +               goto out_unlock;
> +
> +       if (vmf->flags & FAULT_FLAG_PREFAULT_OLD) {
> +               pte_t pte = pte_mkyoung(*vmf->pte);
> +               if (ptep_set_access_flags(vmf->vma, address, vmf->pte, pte, 0))
> +                       update_mmu_cache(vmf->vma, address, vmf->pte);
> +       }

Oh, please dear God no.

First you incorrectly set it old, and then you conditionally make it
young again and as a result force an atomic rwm update and another TLB
flush for no good reason.

Just make sure that the FAULT_FLAG_PREFAULT_OLD never sets the
*actual* address to old.

And yes, that probably means that you need to change "alloc_set_pte()"
to actually pass in the real address, and leave "vmf->address" alone -
so that it can know which ones are prefaulted and which one is real,
but that sounds like a good idea anyway.

Then you can just make alloc_set_pte() do the right thing in the first
place, instead of doing this nasty "lets do it wrong and fix it up
later" horror.

                Linus

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2020-12-09 17:59 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-09 16:39 [PATCH 0/2] Create 'old' ptes for faultaround mappings on arm64 with hardware access flag Will Deacon
2020-12-09 16:39 ` [PATCH 1/2] mm: Allow architectures to request 'old' entries when prefaulting Will Deacon
2020-12-09 17:58   ` Linus Torvalds [this message]
2020-12-09 18:40     ` Will Deacon
2020-12-09 19:04       ` Linus Torvalds
2020-12-09 20:32         ` Matthew Wilcox
2020-12-09 21:04           ` Linus Torvalds
2020-12-10 15:08         ` Kirill A. Shutemov
2020-12-10 17:23           ` Linus Torvalds
2020-12-14 16:07             ` Kirill A. Shutemov
2020-12-14 17:54               ` Linus Torvalds
2020-12-14 18:56                 ` Matthew Wilcox
2020-12-16 17:07                 ` Kirill A. Shutemov
2020-12-16 18:41                   ` Linus Torvalds
2020-12-17 10:54                     ` Kirill A. Shutemov
2020-12-17 18:22                       ` Linus Torvalds
2020-12-18 11:04                         ` Kirill A. Shutemov
2020-12-18 18:56                           ` Linus Torvalds
2020-12-19 12:41                             ` Kirill A. Shutemov
2020-12-19 20:08                               ` Linus Torvalds
2020-12-19 20:34                               ` Linus Torvalds
2020-12-22 10:00                                 ` Kirill A. Shutemov
2020-12-24  4:04                                   ` Hugh Dickins
2020-12-25 11:31                                     ` Kirill A. Shutemov
2020-12-26 17:57                                       ` Linus Torvalds
2020-12-26 20:43                                         ` Kirill A. Shutemov
2020-12-26 21:03                                           ` Hugh Dickins
2020-12-26 21:16                                             ` Linus Torvalds
2020-12-26 22:40                                               ` Kirill A. Shutemov
2020-12-27  0:45                                                 ` Hugh Dickins
2020-12-27  2:38                                                   ` Hugh Dickins
2020-12-27 19:38                                                     ` Linus Torvalds
2020-12-27 20:32                                                       ` Damian Tometzki
2020-12-27 22:35                                                         ` Hugh Dickins
2020-12-27 23:12                                                           ` Linus Torvalds
2020-12-27 23:40                                                             ` Linus Torvalds
2020-12-27 23:55                                                               ` Kirill A. Shutemov
2020-12-27 23:48                                                       ` Kirill A. Shutemov
2020-12-28  1:54                                                         ` Linus Torvalds
2020-12-28  6:43                                                           ` Hugh Dickins
2020-12-28 12:53                                                             ` Kirill A. Shutemov
2020-12-28 18:47                                                               ` Linus Torvalds
2020-12-28 21:58                                                                 ` Linus Torvalds
2020-12-29 13:28                                                                   ` Kirill A. Shutemov
2020-12-29 15:19                                                                     ` Matthew Wilcox
2020-12-29 20:52                                                                     ` Linus Torvalds
2020-12-28 22:05                                                                 ` Kirill A. Shutemov
2020-12-28 22:12                                                                   ` Kirill A. Shutemov
2020-12-29  4:35                                                                     ` Hugh Dickins
2020-12-28 23:28                                                                   ` Linus Torvalds
2020-12-26 21:07                                           ` Linus Torvalds
2020-12-26 21:41                                           ` Matthew Wilcox
2020-12-09 16:39 ` [PATCH 2/2] arm64: mm: Implement arch_wants_old_faultaround_pte() Will Deacon
2020-12-09 18:35   ` Catalin Marinas
2020-12-09 18:46     ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgos=vgteG52=J=rVSeq6-Y2g2+Kn1=xV=wYjVzM6O9UQ@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=catalin.marinas@arm.com \
    --cc=jack@suse.cz \
    --cc=kernel-team@android.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=vinmenon@codeaurora.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).